Batcher: Removed half pixel offset calculation.

[stallratte]

Hello,

when switching from Monogame to FNA, I experienced some kind of texture bleeding:

FNA

I compared the NEZ Batcher with the FNA Batcher.
(Kinda hard to read because of pointer arithmetic)

https://github.com/FNA-XNA/FNA/blob/master/src/Graphics/SpriteBatch.cs#L1436-L1457

// transformMatrix = Matrix.Identity
// notice the memory layout
M11	dstPtr[0] = (tfWidth * 1) - 0;			// tfWidth => (float) (2.0 / (double) viewport.Width)
M21	dstPtr[1] = (tfWidth * 0) - 0;
M31	dstPtr[2] = (tfWidth * 0) - 0;
M41	dstPtr[3] = (tfWidth * 0) - 1;			// -1
M12	dstPtr[4] = (tfHeight * 0) + 0;
M22	dstPtr[5] = (tfHeight * 1) + 0;			// tfHeight = (float) (-2.0 / (double) viewport.Height)
M32	dstPtr[6] = (tfHeight * 0) + 0;
M42	dstPtr[7] = (tfHeight * 0) + 1;			// 1
M13	dstPtr[8] = 0;
M23	dstPtr[9] = 0;
M33	dstPtr[10] = 0;
M43	dstPtr[11] = 0;
M14	dstPtr[12] = 0;
M24	dstPtr[13] = 0;
M34	dstPtr[14] = 0;
M44	dstPtr[15] = 1;							// 1

Key takeaway:
M11 = (float)(2.0 / (double)viewport.Width)
M22 = (float)(-2.0 / (double)viewport.Height)
M41 = -1
M42 = 1

So no half pixel offset is calculated in the SpriteBatcher of FNA.

This is also mentioned here: https://fna-xna.github.io/docs/2a%3A-Building-XNA-Games-with-FNA/#2a-about-effect-support

We do not attempt to undo half-pixel offsets applied by the game's shaders. If you don't know what this is, don't worry. If you do know what this is and have applied them in your engine, simply remove them from the FNA version of your game and it should visually match D3D without any problems.

(I initially got confused because of https://github.com/FNA-XNA/FNA/blob/master/src/Graphics/Effect/StockEffects/SpriteEffect.cs#L71-L74
But SpriteEffect is not used anywhere and is an internal class. So the offset calculation is never applied anywhere.)

Monogame

I also checked the SpriteBatcher of Monogame.
It also does not apply an offset by default:

• https://github.com/MonoGame/MonoGame/blob/develop/MonoGame.Framework/Graphics/SpriteBatch.cs#L100
• https://github.com/MonoGame/MonoGame/blob/develop/MonoGame.Framework/Graphics/Effect/SpriteEffect.cs#L71-L85

But it does if 'UseHalfPixelOffset' is set to true. So they still support this feature (for OpenGL):
https://docs.monogame.net/articles/migration/migrate_xna.html

For DirectX the flag will be set to false: https://github.com/MonoGame/MonoGame/blob/develop/MonoGame.Framework/Graphics/GraphicsDevice.cs#L317-L320

Key takeaways from the article:

1. This flag is set to false by default to encourage users to use the modern style of pixel addressing.
2. SpriteBatch rendering is not affected by the flag.

Changes

Completely removed the half pixel offset calculation in the Batcher.
This benefits FNA and Monogame. For both frameworks this calculation should not be used.
The problems are more visible when using FNA but M41/M42 is also a little bit off for Monogame.

Set the Monogame-specific 'UseHalfPixelOffset'-Flag in Core to false. I do not think many users will need this feature. And having it set to true by default will do more harm than good. That's also the reason why this flag is set to false by default in monogame.

I am aware that legacy XNA games in Monogame will now have the half pixel offset problem. But to fix this, it would be necessary to check for more conditions aside from 'UseHalfPixelOffset'.
And I don't think adding this legacy feature is worth the time.

I did test this change with FNA and Monogame (OpenGL only) and in both cases I didn't see any texture bleeding.

Best regards,
stallratte

[optimo]

I gave this a spin on my setup: linux + FNA opengl. And it does work and fixes some of weird issues I've seen and mildly reported over the years.

I do use gapless tileset for my tiled maps and this fixes the extra pixels near the boundaries that I've witnessed.

More apparently it fixes a quirk i'd witnessed many times with bitmap font sprite rendering near screen edges in a screenspace renderer. if I didnt give the text labels a padding of at least 1 to get away from the top edge, the label would bleed an extra pixel.

Included screenshots of this fix working on the nezui label font.

Notably: my font atlas is gapless. However, this bleeding also occurs with the default nez bitmap font (suppose it is also gapless -- I believe the BMFont et al tools default to exporting gapless fwiw - but nevertheless I've witnessed this bleeding on my setup since the very beginning regardless of which font)

Edit: I've removed the extra paragraphs here about the weird padding I noticed in text rendering; fonts seemingly having a 1pixel gap baked-in that I've only noticed now. However, as stallrate mentioned in chat, this 'yoffset' padding seems to originate in the bmfont file and so nez is rendering it accurately as per spec; this fix was not a cause of any extra weirdness. It gets my thumbs-up overall.

[optimo]

I've found some kind of rendering issue introduced by this change. I will provide more info soon.
...
Okay. I had a hunch because this problem was appearing with one of my animated sprites.
The sprite has an odd-number width. This seems to be the cause of the issue with it rending weirdly. Tested making the art one pixel wider and that does make the issue go away. But probably this is showing some kind of inconsistency in the new maths?

[stallratte]

Hey @optimo,

thank you for your feedback.

But probably this is showing some kind of inconsistency in the new maths?

Imo it's the same as in the FNA SpriteBatch.

The sprite has an odd-number width.

I always use even-numbered sprites, so I didn't run into this issue.
And I do think it's best to use even-numbered sprites. Especially for animated / moving sprites.

I did a test anyway with a 9x9 texture.

To my understanding, if you really need to use an odd-numbered sprite, you should adjust the position accordingly so it will end up with an integer raster position.

Here's an example to show what I mean:

public class SampleScene : Scene {

	private Entity _spriteEntity;

	public override void Initialize() {
		SetDesignResolution(640, 360, SceneResolutionPolicy.ShowAllPixelPerfect);

		var texture = Content.LoadTexture("Content/sprite9x9.png");

		var sourceRectangle = texture.Bounds;

		var centerOrigin = sourceRectangle.GetHalfSize();

		var sprite = new Sprite(texture, sourceRectangle, centerOrigin);

		_spriteEntity = CreateEntity("sprite");
		var spriteRenderer = _spriteEntity.AddComponent(new SpriteRenderer(sprite));
		spriteRenderer.SetLocalOffset(new Vector2(0.5f, 0.5f));
		_spriteEntity.Position = new Vector2(100, 100);
	}

	public override void Update() {
		base.Update();

		if (Input.IsKeyDown(Keys.Down)) {
			_spriteEntity.Position = new Vector2(_spriteEntity.Position.X, _spriteEntity.Position.Y + 1);
		}
		else if (Input.IsKeyDown(Keys.Up)) {
			_spriteEntity.Position = new Vector2(_spriteEntity.Position.X, _spriteEntity.Position.Y - 1);
		}
	}

}

The default origin when using a Sprite is center.

Position: (100, 100)
Center: (4.5, 4.5)

If we do not compensate the 'odd numbered texture' by updating the position with e.g.
spriteRenderer.SetLocalOffset(new Vector2(0.5f, 0.5f));
we will not get an integer for e.g. the top left corner.

Instead it will have this position: (95.5, 95,5)

As can be seen here:

https://github.com/prime31/Nez/blob/master/Nez.Portable/Graphics/Batcher/Batcher.cs#L901-L914

And this will result in the sprite not being rendered correctly.

This could normally be fixed by updating the position. But there is a flag which prevents that: 'ShouldRoundDestinations'
https://github.com/prime31/Nez/blob/master/Nez.Portable/Graphics/Batcher/Batcher.cs#L24C15-L24C46

I don't know why this flag exists, but it prevents the user from manually adjusting the position: https://github.com/prime31/Nez/blob/master/Nez.Portable/Graphics/Batcher/Batcher.cs#L855-L859

So this flag needs to be set to false. I really do wonder why this flag is set to true by default? (Looks like it was introduced with #367)

Here's the result of the calculation:
Top left is now (96, 96) instead of (95.5, 95.5)

The sprite is now being displayed correctly.

Alternatively it would also be possible to just set the origin to Vector2.Zero.

Here's the texture I used in case you also want to experiment a bit:

[optimo]

Very good research. Understood and all good. Setting the sprite origin to Vector2.Zero actually best matches my expectations for how sprites should work. So that's how I will plan to use them in the future also. This was a good think-about and reinforces my suspicions about default sprite origins being kind of annoying for my use-case.

Since this problem starts in the atlas being generated for me by the Aseprite class, I'm going to introduce a PR adding a method there to allow setting a custom origin for the created sprites. This seems like the best remedy. The aseprite methods have only been in nez framework for a short while. Others using manually-created sequences or atlases have otherwise full agency over the origin settings if need be.

The rounding of destinations you uncovered holds some intrigue but we can put that aside for now. It's another can of worms. :)