Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grayscale textures are broken in 4.2.0.2188 #2376

Closed
Doprez opened this issue Jul 7, 2024 · 11 comments
Closed

Grayscale textures are broken in 4.2.0.2188 #2376

Doprez opened this issue Jul 7, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@Doprez
Copy link
Contributor

Doprez commented Jul 7, 2024

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
Copy link
Contributor Author

Doprez commented Jul 8, 2024

Some added info this seems to have broken after 2149 as it works in that gamestudio version as expected.
releases/4.2.0.2149...releases/4.2.0.2188

@Doprez
Copy link
Contributor Author

Doprez commented Jul 8, 2024

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
Copy link
Collaborator

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

@Doprez
Copy link
Contributor Author

Doprez commented Jul 8, 2024

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 #2303 was merged and now it works.

@Jklawreszuk
Copy link
Collaborator

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

@tebjan
Copy link
Member

tebjan commented Jul 9, 2024

@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
Copy link
Member

tebjan commented Jul 9, 2024

@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
Copy link
Contributor Author

Doprez commented Jul 9, 2024

@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
Copy link
Collaborator

@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
Copy link
Contributor

Basewq commented Aug 21, 2024

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:

ChangeDxtImageType(libraryData, (DXGI_FORMAT)(loader.LoadAsSRgb ? format.ToSRgb() : format.ToNonSRgb()));

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):

var libraryData = new FreeImageTextureLibraryData { Bitmaps = new [] { FreeImage.ConvertTo32Bits(temp) } };

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.

@Eideren
Copy link
Collaborator

Eideren commented Sep 14, 2024

Closed in favor of the more specific #2449

@Eideren Eideren closed this as completed Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants