[2.1] Pixel format mismatch for monochrome textures in D3D11

Discussion area about developing with Ogre2 branches (2.1, 2.2 and beyond)
Post Reply
User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 157
Joined: Wed Apr 23, 2014 3:49 pm
x 16

[2.1] Pixel format mismatch for monochrome textures in D3D11

Post by TaaTT4 » Wed Jun 12, 2019 10:51 am

In commit bfb2f474f85f Eugene introduced a pixel format conversion for D3D11 textures, from PF_L8 to PF_R8G8B8. This causes issues in PBS since monochrome textures (e.g.: roughness and metalness) still relies on PF_L8 pixel format. As is stated in Eugene comments, to avoid the conversion PF_R8 must be used as a default pixel format for monochrome textures.
1 x
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4034
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 214
Contact:

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by dark_sylinc » Thu Jun 13, 2019 5:25 pm

I've been trying to avoid dealing with this issue for years. Using 4 bytes for L8 breaks a lot of user code too (because they don't expect L8 to have stride of 4 bytes per pixel. Theoretically they should've check the stride, but not everyone does)

I asked Eugene if we really need this because in Ogre 2.1 everything is shader based, and our Hlms implementations are aware of the problem (e.g. HlmsUnlitDatablock::setTextureSwizzle can fix this).

Using R8 is problematic because there's a lot of untested code going on. Particularly GL3+ didn't even implement R8.
This is what I got so far:

Code: Select all

diff -r 53e01f4f2e9d OgreMain/src/OgreHlmsTextureManager.cpp
--- a/OgreMain/src/OgreHlmsTextureManager.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/OgreMain/src/OgreHlmsTextureManager.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -49,7 +49,7 @@
     HlmsTextureManager::HlmsTextureManager() : mRenderSystem( 0 ), mTextureId( 0 )
     {
         mDefaultTextureParameters[TEXTURE_TYPE_DIFFUSE].hwGammaCorrection   = true;
-        mDefaultTextureParameters[TEXTURE_TYPE_MONOCHROME].pixelFormat      = PF_L8;
+        mDefaultTextureParameters[TEXTURE_TYPE_MONOCHROME].pixelFormat      = PF_R8;
         mDefaultTextureParameters[TEXTURE_TYPE_NORMALS].pixelFormat         = PF_BC5_SNORM;
         mDefaultTextureParameters[TEXTURE_TYPE_NORMALS].isNormalMap         = true;
         mDefaultTextureParameters[TEXTURE_TYPE_DETAIL].hwGammaCorrection    = true;
diff -r 53e01f4f2e9d OgreMain/src/OgreImage.cpp
--- a/OgreMain/src/OgreImage.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/OgreMain/src/OgreImage.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -608,6 +608,7 @@
         switch( mFormat )
         {
         case PF_L8:
+        case PF_R8:
             if( !gammaCorrected )
             {
                 downsampler2DFunc   = downscale2x_X8;
@@ -934,7 +935,7 @@
         case FILTER_BILINEAR:
             switch (src.format) 
             {
-            case PF_L8: case PF_A8: case PF_BYTE_LA:
+            case PF_L8: case PF_R8: case PF_A8: case PF_BYTE_LA:
             case PF_R8G8B8: case PF_B8G8R8:
             case PF_R8G8B8A8: case PF_B8G8R8A8:
             case PF_A8B8G8R8: case PF_A8R8G8B8:
diff -r 53e01f4f2e9d OgreMain/src/OgrePixelConversions.h
--- a/OgreMain/src/OgrePixelConversions.h	Sat Jun 08 15:53:17 2019 -0300
+++ b/OgreMain/src/OgrePixelConversions.h	Thu Jun 13 13:22:17 2019 -0300
@@ -201,6 +201,54 @@
     }
 };
 
+struct A8B8G8R8toR8: public PixelConverter <Ogre::uint32, Ogre::uint8, FMTCONVERTERID(Ogre::PF_A8B8G8R8, Ogre::PF_R8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return (Ogre::uint8)(inp&0x000000FF);
+    }
+};
+
+struct R8toA8B8G8R8: public PixelConverter <Ogre::uint8, Ogre::uint32, FMTCONVERTERID(Ogre::PF_R8, Ogre::PF_A8B8G8R8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return 0xFF000000|(((unsigned int)inp)<<0)|(((unsigned int)inp)<<8)|(((unsigned int)inp)<<16);
+    }
+};
+
+struct A8R8G8B8toR8: public PixelConverter <Ogre::uint32, Ogre::uint8, FMTCONVERTERID(Ogre::PF_A8R8G8B8, Ogre::PF_R8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return (Ogre::uint8)((inp&0x00FF0000)>>16);
+    }
+};
+
+struct R8toA8R8G8B8: public PixelConverter <Ogre::uint8, Ogre::uint32, FMTCONVERTERID(Ogre::PF_R8, Ogre::PF_A8R8G8B8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return 0xFF000000|(((unsigned int)inp)<<0)|(((unsigned int)inp)<<8)|(((unsigned int)inp)<<16);
+    }
+};
+
+struct B8G8R8A8toR8: public PixelConverter <Ogre::uint32, Ogre::uint8, FMTCONVERTERID(Ogre::PF_B8G8R8A8, Ogre::PF_R8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return (Ogre::uint8)((inp&0x0000FF00)>>8);
+    }
+};
+
+struct R8toB8G8R8A8: public PixelConverter <Ogre::uint8, Ogre::uint32, FMTCONVERTERID(Ogre::PF_R8, Ogre::PF_B8G8R8A8)>
+{
+    inline static DstType pixelConvert(SrcType inp)
+    {
+        return 0x000000FF|(((unsigned int)inp)<<8)|(((unsigned int)inp)<<16)|(((unsigned int)inp)<<24);
+    }
+};
+
 struct A8B8G8R8toL8: public PixelConverter <Ogre::uint32, Ogre::uint8, FMTCONVERTERID(Ogre::PF_A8B8G8R8, Ogre::PF_L8)>
 {
     inline static DstType pixelConvert(SrcType inp)
@@ -398,6 +446,12 @@
         CASECONVERTER(R8G8B8A8toA8R8G8B8);
         CASECONVERTER(R8G8B8A8toA8B8G8R8);
         CASECONVERTER(R8G8B8A8toB8G8R8A8);
+        CASECONVERTER(A8B8G8R8toR8);
+        CASECONVERTER(R8toA8B8G8R8);
+        CASECONVERTER(A8R8G8B8toR8);
+        CASECONVERTER(R8toA8R8G8B8);
+        CASECONVERTER(B8G8R8A8toR8);
+        CASECONVERTER(R8toB8G8R8A8);
         CASECONVERTER(A8B8G8R8toL8);
         CASECONVERTER(L8toA8B8G8R8);
         CASECONVERTER(A8R8G8B8toL8);
diff -r 53e01f4f2e9d RenderSystems/Direct3D11/src/OgreD3D11Mappings.cpp
--- a/RenderSystems/Direct3D11/src/OgreD3D11Mappings.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/RenderSystems/Direct3D11/src/OgreD3D11Mappings.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -471,7 +471,7 @@
         case DXGI_FORMAT_R16_SNORM:                 return PF_R16_SNORM;
         case DXGI_FORMAT_R16_SINT:                  return PF_R16_SINT;
         case DXGI_FORMAT_R8_TYPELESS:               return PF_UNKNOWN;
-        case DXGI_FORMAT_R8_UNORM:                  return PF_L8;
+        case DXGI_FORMAT_R8_UNORM:                  return PF_R8;
         case DXGI_FORMAT_R8_UINT:                   return PF_UNKNOWN;
         case DXGI_FORMAT_R8_SNORM:                  return PF_UNKNOWN;
         case DXGI_FORMAT_R8_SINT:                   return PF_UNKNOWN;
@@ -538,6 +538,7 @@
     {
         switch(ogrePF)
         {
+        case PF_R8:             return DXGI_FORMAT_R8_UNORM;
         case PF_L8:             return DXGI_FORMAT_R8_UNORM;
         case PF_L16:            return DXGI_FORMAT_R16_UNORM;
         case PF_A8:             return DXGI_FORMAT_A8_UNORM;
diff -r 53e01f4f2e9d RenderSystems/Direct3D11/src/OgreD3D11PixelFormatToShaderType.cpp
--- a/RenderSystems/Direct3D11/src/OgreD3D11PixelFormatToShaderType.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/RenderSystems/Direct3D11/src/OgreD3D11PixelFormatToShaderType.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -35,7 +35,6 @@
         switch( pixelFormat )
         {
         //UNORM formats
-        case PF_L8:
         case PF_A8:
         case PF_R8:
         case PF_D16_UNORM:
@@ -50,6 +49,7 @@
             return "unorm float2";
         case PF_SHORT_GR:
             return "unorm float2";
+        case PF_L8:
         case PF_R8G8B8:
         case PF_B8G8R8:
         case PF_A8R8G8B8:
diff -r 53e01f4f2e9d RenderSystems/GL3Plus/src/OgreGL3PlusPixelFormat.cpp
--- a/RenderSystems/GL3Plus/src/OgreGL3PlusPixelFormat.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/RenderSystems/GL3Plus/src/OgreGL3PlusPixelFormat.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -59,6 +59,7 @@
         case PF_X32_X24_S8_UINT:
             return GL_DEPTH_STENCIL;
         case PF_A8:
+        case PF_R8:
         case PF_L8:
         case PF_L16:
         case PF_R8_SNORM:
@@ -189,6 +190,7 @@
         switch(format)
         {
         case PF_BYTE_LA:
+        case PF_R8:
         case PF_A8:
         case PF_L8:
         case PF_R8G8B8:
@@ -328,6 +330,7 @@
         case PF_D32_FLOAT_X24_X8:
         case PF_X32_X24_S8_UINT:
             return GL_DEPTH32F_STENCIL8;
+        case PF_R8:
         case PF_L8:
         case PF_A8:
             return GL_R8;
@@ -628,7 +631,7 @@
         case GL_DEPTH32F_STENCIL8:
             return PF_D32_FLOAT_X24_S8_UINT;
         case GL_R8:
-            return PF_L8;
+            return PF_R8;
         case GL_R16:
             return PF_L16;
         case GL_RG: //TODO Is there a better OGRE format?
diff -r 53e01f4f2e9d Samples/2.0/ApiUsage/AreaApproxLights/AreaApproxLightsGameState.cpp
--- a/Samples/2.0/ApiUsage/AreaApproxLights/AreaApproxLightsGameState.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/Samples/2.0/ApiUsage/AreaApproxLights/AreaApproxLightsGameState.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -50,7 +50,7 @@
     {
         const Ogre::uint32 texWidth = 256u;
         const Ogre::uint32 texHeight = 256u;
-        const Ogre::PixelFormat texFormat = Ogre::PF_L8;
+        const Ogre::PixelFormat texFormat = Ogre::PF_R8;
 
         //Fill the texture with a hollow rectangle, 10-pixel thick.
         size_t sizeBytes = Ogre::PixelUtil::calculateSizeBytes(
diff -r 53e01f4f2e9d Samples/2.0/Showcase/Postprocessing/PostprocessingGameState.cpp
--- a/Samples/2.0/Showcase/Postprocessing/PostprocessingGameState.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/Samples/2.0/Showcase/Postprocessing/PostprocessingGameState.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -73,7 +73,7 @@
             TEX_TYPE_3D,
             64,64,64,
             0,
-            PF_L8,
+            PF_R8,
             TU_DYNAMIC_WRITE_ONLY
         );
 
@@ -117,7 +117,7 @@
             TEX_TYPE_2D,
             renderWindow->getWidth(), renderWindow->getHeight() , 1,
             0,
-            PF_L8,
+            PF_R8,
             TU_DYNAMIC_WRITE_ONLY
         );
 
diff -r 53e01f4f2e9d Tests/OgreMain/src/PixelFormatTests.cpp
--- a/Tests/OgreMain/src/PixelFormatTests.cpp	Sat Jun 08 15:53:17 2019 -0300
+++ b/Tests/OgreMain/src/PixelFormatTests.cpp	Thu Jun 13 13:22:17 2019 -0300
@@ -204,6 +204,13 @@
     testCase(PF_R8G8B8A8,PF_A8B8G8R8);
     testCase(PF_R8G8B8A8,PF_B8G8R8A8);
 
+    testCase(PF_A8B8G8R8, PF_R8);
+    testCase(PF_R8, PF_A8B8G8R8);
+    testCase(PF_A8R8G8B8, PF_R8);
+    testCase(PF_R8, PF_A8R8G8B8);
+    testCase(PF_B8G8R8A8, PF_R8);
+    testCase(PF_R8, PF_B8G8R8A8);
+
     testCase(PF_A8B8G8R8, PF_L8);
     testCase(PF_L8, PF_A8B8G8R8);
     testCase(PF_A8R8G8B8, PF_L8);

But it would require extensive testing. I am leaning more towards the previous behavior (force L8 to be monochrome and fix it shader side) unless someone volunteers for the extensive testing on all 4 platforms (GL3+, D3D11, Metal on mac, Metal on iOS).

I don't have the time for such thorough testing, and all these interfaces are disappearing in Ogre 2.2
0 x

paroj
OGRE Team Member
OGRE Team Member
Posts: 835
Joined: Sun Mar 30, 2014 2:51 pm
x 147
Contact:

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by paroj » Thu Jun 13, 2019 8:49 pm

your R8 to something conversions are wrong, see https://github.com/OGRECave/ogre/pull/1215
0 x

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 157
Joined: Wed Apr 23, 2014 3:49 pm
x 16

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by TaaTT4 » Thu Jun 13, 2019 10:41 pm

Uao, I didn't imagine I would uncovered such a Pandora's box! :twisted:
0 x
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 157
Joined: Wed Apr 23, 2014 3:49 pm
x 16

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by TaaTT4 » Mon Jun 17, 2019 9:17 am

dark_sylinc wrote:
Thu Jun 13, 2019 5:25 pm
Using 4 bytes for L8 breaks a lot of user code too (because they don't expect L8 to have stride of 4 bytes per pixel. Theoretically they should've check the stride, but not everyone does)

I asked Eugene if we really need this because in Ogre 2.1 everything is shader based, and our Hlms implementations are aware of the problem (e.g. HlmsUnlitDatablock::setTextureSwizzle can fix this).

Using R8 is problematic because there's a lot of untested code going on. Particularly GL3+ didn't even implement R8.

I am leaning more towards the previous behavior (force L8 to be monochrome and fix it shader side) unless someone volunteers for the extensive testing on all 4 platforms (GL3+, D3D11, Metal on mac, Metal on iOS).
So, what do you suggest Matias? Consider that I'm interested just in D3D11 render system.
Replacing PF_L8 with PF_R8 in HlmsTextureManager::mDefaultTextureParameters seems to (at least visually!) fix the issues. But I'm open to revert Eugene modifications if it's a more proper/sane solution.
0 x
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4034
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 214
Contact:

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by dark_sylinc » Mon Jun 17, 2019 4:05 pm

I talked with Eugene, it looks like R8 will have to stay.

I'd suggest to apply the patch, while also fixing whatever paroj detected that was wrong.
0 x

paroj
OGRE Team Member
OGRE Team Member
Posts: 835
Joined: Sun Mar 30, 2014 2:51 pm
x 147
Contact:

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by paroj » Mon Jun 17, 2019 4:22 pm

the "struct R8toA8B8G8R8" converters do luminance conversions instead of writing to the red channel only in the patch you posted.
0 x

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 157
Joined: Wed Apr 23, 2014 3:49 pm
x 16

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by TaaTT4 » Mon Jun 17, 2019 5:12 pm

dark_sylinc wrote:
Mon Jun 17, 2019 4:05 pm
I talked with Eugene, it looks like R8 will have to stay.

I'd suggest to apply the patch, while also fixing whatever paroj detected that was wrong.
I will do it. Thanks!
paroj wrote:
Mon Jun 17, 2019 4:22 pm
the "struct R8toA8B8G8R8" converters do luminance conversions instead of writing to the red channel only in the patch you posted.
This commit contains the patch that Matias posted above (except the HlmsTextureManager part which doesn't exist in OGRE 1.x) plus the fix you talk about. Is it correct @paroj?
0 x
Senior game programmer at Vae Victis
Working on Racecraft

paroj
OGRE Team Member
OGRE Team Member
Posts: 835
Joined: Sun Mar 30, 2014 2:51 pm
x 147
Contact:

Re: [2.1] Pixel format mismatch for monochrome textures in D3D11

Post by paroj » Mon Jun 17, 2019 5:58 pm

TaaTT4 wrote:
Mon Jun 17, 2019 5:12 pm
paroj wrote:
Mon Jun 17, 2019 4:22 pm
the "struct R8toA8B8G8R8" converters do luminance conversions instead of writing to the red channel only in the patch you posted.
This commit contains the patch that Matias posted above (except the HlmsTextureManager part which doesn't exist in OGRE 1.x) plus the fix you talk about. Is it correct @paroj?
yes, I just cherry-picked it to v2-1 as it applies cleanly
0 x

Post Reply