stride3d / stride

Stride Game Engine (formerly Xenko)
https://stride3d.net
MIT License
6.42k stars 931 forks source link

Grayscale textures are broken in 4.2.0.2188 #2376

Open Doprez opened 1 month ago

Doprez commented 1 month ago

Release Type: Official Release

Version: 4.2.0.2188

Platform(s): Windows

Describe the bug When importing a grayscale file into the editor it will be completely broken.

image

you can make it at least shows something if you uncheck all of the options in the feature panel but it just shows white noise.

image

This is how it looks in 4.2.0.2122:

image

To Reproduce Steps to reproduce the behavior:

  1. get a grayscale image BrickWall02_roughness

  2. import it into the editor

Expected behavior Textures should look good.

Doprez commented 1 month ago

Some added info this seems to have broken after 2149 as it works in that gamestudio version as expected. https://github.com/stride3d/stride/compare/releases/4.2.0.2149...releases/4.2.0.2188

Doprez commented 1 month ago

System.DivideByZeroException: 'Attempted to divide by zero.'

This exception was originally thrown at this call stack:
    Stride.Graphics.PixelFormatExtensions.SizeInBits(Stride.Graphics.PixelFormat) in PixelFormatExtensions.cs
    Stride.TextureConverter.TexLibraries.DxtTexLib.ChangeDxtImageType(Stride.TextureConverter.TexLibraries.DxtTextureLibraryData, Stride.TextureConverter.DxtWrapper.DXGI_FORMAT) in DxtTexLib.cs
    Stride.TextureConverter.TexLibraries.DxtTexLib.Load(Stride.TextureConverter.TexImage, Stride.TextureConverter.Requests.LoadingRequest) in DxtTexLib.cs
    Stride.TextureConverter.TexLibraries.DxtTexLib.Execute(Stride.TextureConverter.TexImage, Stride.TextureConverter.IRequest) in DxtTexLib.cs
    Stride.TextureConverter.TextureTool.Load(Stride.TextureConverter.Requests.LoadingRequest) in TextureTool.cs
    Stride.TextureConverter.TextureTool.Load(string, bool) in TextureTool.cs
    Stride.Assets.Textures.TextureAssetCompiler.TextureConvertCommand.DoCommandOverride(Stride.Core.BuildEngine.ICommandContext) in TextureAssetCompiler.cs
    Stride.Core.BuildEngine.Command.DoCommand(Stride.Core.BuildEngine.ICommandContext) in Command.cs
    Stride.Core.BuildEngine.CommandBuildStep.StartCommand(Stride.Core.BuildEngine.IExecuteContext, Stride.Core.IO.ListStore<Stride.Core.BuildEngine.CommandResultEntry>, Stride.Core.BuildEngine.BuilderContext) in CommandBuildStep.cs
Jklawreszuk commented 1 month ago

Weird, Are being able to reproduce this bug before my PR related to DirectXTex? https://github.com/stride3d/stride/pull/2338

Doprez commented 1 month ago

Weird, Are being able to reproduce this bug before my PR related to DirectXTex? #2338

It seems like it may be before that change.

I just pulled a headless commit before https://github.com/stride3d/stride/pull/2303 was merged and now it works.

Jklawreszuk commented 1 month ago

@Doprez Could you upload tested texture here in this issue? I wonder what gone wrong that it executes DxtTextLib in the first place...

tebjan commented 1 month ago

@Jklawreszuk it is the one in the post above, probably it has a high bit format. Maybe 16, 24 or 32 bit per channel. I can confirm that this image doesn't work: Bricks

tebjan commented 1 month ago

@Jklawreszuk what was the reason to change the supported file formats in #2303? The DxtLib handled png files with esoteric pixel formats correctly. There might also be gamma/sRGB issues that are handled differently...

https://github.com/stride3d/stride/pull/2303/files#diff-04a405ef92d4c86abd7af1eb0d6e45b71ba305394925e5922bcf0131f22a4c7eL47

Doprez commented 1 month ago

@Doprez Could you upload tested texture here in this issue? I wonder what gone wrong that it executes DxtTextLib in the first place...

It should be the image in the description, unfortunately it shows the picture instead of just a file reference but opening athat and downloading it should reproduce the issue.

Jklawreszuk commented 1 month ago

@Doprez Right, thanks! I didn't pay attention to it 🤦 @tebjan LoadFromWICFile depends on Win32 which is available on Windows only, that's why was gotten removed.

DirectXTex wiki : https://github.com/microsoft/DirectXTex/wiki/WIC-I-O-Functions.

So intention was to move responsibility of image loading from dxt to freeimage but I guess modifying SupportedExtensions variable isn't enough. Will try this week to see what needs to be changed additionally to make the import work properly. If I don't find any idea to fix, restoring the LoadFromWICFile method while adding a workaround for Linux systems may also be some solution to the problem 😅

Basewq commented 1 day ago

Not urgent for me, but is there any progress on this issue?

From what I can tell, DxtTexLib loads the images in whatever pixel format and only changes DxtTexLib would make is converting srgb/non-srgb: https://github.com/stride3d/stride/blob/6a2e29b6aa4aeaf852631f9466b05b201d182142/sources/tools/Stride.TextureConverter/Backend/TexLibraries/DxtTexLib.cs#L247 eg. That 16-bit grayscale image gets loaded as a 16-bit pixel format texture.

FITexLib, on the other hand would try to force the loaded image as 32-bit pixel format (BGRA_8888): https://github.com/stride3d/stride/blob/dad92aaa2bd294ae302261fdde52583ab329a6f5/sources/tools/Stride.TextureConverter/Backend/TexLibraries/FITexLib.cs#L246 The 16-bit grayscale image above just fails to get converted (ie. returns as null data), and it basically falls apart from then on.

The solution seems to be either update FITexLib to somehow retain the original pixelformat, or just revert it until someone knows how to fix it through FITexLib (not me).

I haven't tested to see, but I am also concerned that Execute method in DxtTexLib has things that FITexLib doesn't do (MipMapsGeneration, NormalMapGeneration). Not sure if FITexLib not implementing those means it'll get sorted out somewhere else or not.