There is a rendering difference between the DirectX and OpenGL versions of MonoGame when using the BasicEffect. Because the half pixel offset for OpenGL is not taken into account. While this is taken into account for the SpriteBatch:
https://github.com/MonoGame/MonoGame/blob/faea8a6a89504673e2bb7e435f0da8cc513d8c30/Mono
Game.Framework/Graphics/GraphicsDevice.OpenGL.cs#L272
This means that anything rendered with the BasicEffect shader is slightly blurry on OpenGL. For example see these two screenshots:
OpenGL:
DirectX:
I think we should give the user some way to know that the half pixel offset should be set by themselves or change the OpenGL shader for the BasicEffect to fix this.
What do you think? I can make a PR once I've gathered more input :).
We could use preprocessing directives to put OpenGL specific stuff in .fx files with #ifdef OPENGL and rebuild BasicEffect.ogl.mgfxo?
(ImGUI FTW)
This means that anything rendered with the BasicEffect shader is slightly blurry on OpenGL
We could likely fix this in BasicEffect.
But i don't know if we can fix this in a generic way for all custom effects. If we could then that solution would work for everything including SpriteBatch and other stock effects.
@Jjagg - I guess this might already be something handled by the HLSL to GLSL cross compiling tech we're looking at using?
Hold up, there is already a half pixel offset being applied for OpenGL and not for DX. The current behavior is completely wrong, but there's an explanation for how it got like this, from my understanding:
DX9 is the weird one out. All other graphics APIs do it 'right'. XNA didn't because it used DX9. Though it applied a half pixel offset for SpriteBatch. So MG technically needs the half-pixel offset on all platforms that do not use DX9 if we want to be XNA compatible. The DX backend doesn't have the offset applied I think because it evolved from DX9 and it was forgotten, though it should to be XNA compatible (looking back this is part of why #5144 failed). XNA applies half a pixel offset in SpriteBatch so DX behavior was consistent there which we prove in testing. For OpenGL the half pixel offset is applied to every effect that goes through MojoShader. Somewhere along the line someone must've thought that OpenGL spritebatch rendering wasn't consistent with DX, due to that half pixel offset for every effect, so a reverse half pixel offset to spritebatch was added.
So bottom line is, we need to get rid of the half pixel offset in SpriteBatch for OpenGL. We need to decide whether we want to force the half pixel offset for all our platforms to better be compatible with XNA or if we want to move forward and get rid of the hack and fix this issue once and for all.
@tomspilman Not sure if any consoles use DX9 API, maybe Vita?
For reference:
@roy-t If we do move away from XNA behavior and make things consistent with our DX implementation, the right fix would be to remove the lines here and leave posFixup[2] and posFixup[3] at 0:
https://github.com/MonoGame/MonoGame/blob/6f34eb393aa0ac005888d74c5c4c6ab5615fdc8c/MonoGame.Framework/Graphics/GraphicsDevice.OpenGL.cs#L938-L939
If we go the other way around, we need to apply the half pixel offset in SpriteBatch on all platforms and somehow fix the vertex position in all shaders. That sounds a lot more painful to me, and considering the non-SpriteBatch parts of MG's DX implementation have been inconsistent with XNA for so long (and it's kind of our reference implementation) I'm strongly in favor of dropping the offset altogether and XNA compatibility in this regard with it.
Thanks for the detailed information. It is a lot more complex than I first thought then. I just thought it was forgotten for the basic effect. I'll hold off on changing anything until there is a consensus :). But after that the fix should be really small it seems.
I say fix it before everyone forgets about it and then it becomes a mess later.
If the concern to maintain compatibility is great then turn it off as well but make it a option that can be turned on.
Add a Dx9LegacyPixelOffsetCompatability bool Property to Game or graphics or some were that the user can just turn on just like we do with IsMouseVisible = true; and the like.
Wrap each of those code parts in a if( Game.Dx9LegacyPixelOffsetCompatability ){ /* do the pixel offset */ } or some similar scheme.
Then you get to frame your cake and eat it too.
_I think this can be bandaided though by passing in your own basic effect and adding a half pixel translation to the projection matrix._
@Jjagg after thinking about it a bit more, I'm a bit confused. So if I target Windows+DirectX do I need to add the half pixel offset to my HLSL shaders, just like in the old XNA/DirectX 9 days? And then if the HLSL shader is auto-translated to GLSL it will add an additional ReverseOffset? Or can I forget about the half pixel offset for DirectX? In that case it seems the behavior is already incompatible with XNA, so it might be OK to break it completely there so we can remove the half pixel entirely?
@roy-t My post was maybe a bit verbose and hard to parse.
The MG OpenGL implementation is consistent with XNA so it'd need half a pixel offset and the MG DX implementation is not consistent with XNA (except for SpriteBatch) so it does not. ImGui doesn't expect a DX9 renderer, so things look blurry when rendered with a DX9 compatible renderer like MG's GL backend.
In that case it seems the behavior is already incompatible with XNA, so it might be OK to break it completely there so we can remove the half pixel entirely?
Yes, that's exactly my point as well. MG DX has been incompatible with XNA ever since it's using DX11, so let's make everything work that way.
Wait, MojoShader adds a pixel offset to vertices, and then MG adds another (doing twice the job), and then the SpriteBatch removes one? Pfeww, funny how it ended up being like this...
@roy-t If we do move away from XNA behavior and make things consistent with our DX implementation, the right fix would be to remove the lines here and leave
posFixup[2]andposFixup[3]at 0.
I've tested that and removed the NeedsHalfPixelOffset from OpenGL and everything looks fine to me. SpriteBatch rendering is 1:1 compared to what it is now.
Consoles don't use half pixel and all have DX11/OpenGL style API (so I would expect them to not be XNA accurate as well).
Wait, MojoShader adds a pixel offset to vertices, and then MG adds another (doing twice the job), and then the SpriteBatch removes one? Pfeww, funny how it ended up being like this...
Not exactly. MojoShader adds the parameter for offset, MG sets it for all effects and then undoes it in SpriteBatch. That's technically correct behavior to be compatible with XNA.
@tomspilman Can you weigh in? I'd like to remove the half pixel offset and be done with this mess. If we remove it altogether we'll get consistent behavior on all platforms, SpriteBatch will render like in XNA and other renders will be offset half a pixel compared to XNA (as it is currently in MG DX).
MojoShader only exposes the parameter, alright!
Do we agree that both posFixup and NeedsHalfPixelOffset need to be removed to have consistent rendering between MG DX and MG GL?
Do we agree that both posFixup and NeedsHalfPixelOffset need to be removed to have consistent rendering between MG DX and MG GL?
Yes, that's a fact. Just waiting for Tom to confirm that breaking XNA compatibility is better than hacking every platform to enforce the half pixel offset.
So don't have time to read everything up above here.
IMO keeping XNA compatibility is critical. We should do that wherever we can.
However we have to move forward and fix XNA's sins too.
Ideally there would be some UseStandardPixelAddressing type of global we can use to "opt-in" to breaking XNA compatibility. We would document it as being something one can disable. Maybe even including it in all the template projects with a nice descriptive comment.
I'm gonna implement the fix for this. We'll add a UseStandardPixelAddressing static boolean property to GraphicsDeviceManager that by default is set to false. When set to false we'll behave like XNA by applying the half-pixel offset, so the default behavior is compatible with XNA. For new projects we'll recommend setting this flag to true with documentation and by setting the flag to true in the project templates.
Leaving the flag to false will render things as they are currently rendered by MG on OpenGL platforms. Setting it to true will render things as they are currently rendered by MG on DX platforms. E.g. for ImGui.NET the flag should be set to true.
Note that this will change the behavior for DX projects after upgrading. For SpriteBatch nothing will change, if UseStandardPixelAddressing is set we'll use a reverse offset in SpriteBatch (as is currently done for OpenGL).
hacks on hack on hack on hacks, time to ditch xna no ?
hacks on hack on hack on hacks, time to ditch xna no ?
The plan is to keep a branch XNA accurate (there are still a bunch of relevant uses), and to move away from it in another one.
@Jjagg - UseStandardPixelAddressing was just a off hand guess at a name. Maybe put a little more thought into it than i did. :)
Alternatively we could go with something less technical that expresses the intent, like Dx9Compatible or XnaCompatible. I like UseStandardPixelAddressing better than HalfPixelOffset or something like that, because it implies that non-XNA compatible is the standard currently.
Awesome, sorry for stirring up the beehive, thanks for picking everything up. Since Jjagg is working on this, I'll try and see if there is another (small) issue that I can dedicate a bit of time to in the coming weeks. Thanks so much!
Awesome, sorry for stirring up the beehive
Thanks for reporting, this was long overdue so a poke was needed :)
Hello there! Because I'm using the MonoGame.Framework.DesktopGL.Core version, at the moment there is no possibility to build it under core from the development branch, whoever can help with the explanation how can I fix this half pixel now? (Through reflection or directly in the fx effect file)
Closing this because the OpenGL side is fixed, will open a new issue for the DX side.