-
Notifications
You must be signed in to change notification settings - Fork 40
Add support for compressed textures to VisRTX and enable DDS loading in TSD #107
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
Conversation
Only supporting BC1,2,3,4,5 Bc6 and BC7 might happen to work but are not tested.
Correctly following the specification. This way both VisRTX and VisGL are able to correctly digest the same content.
jeffamstutz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor notes about the GPU-side of things. Let me know if they make sense or not.
devices/rtx/gpu/gpu_objects.h
Outdated
| vec3 invSize; | ||
| }; | ||
|
|
||
| struct CompressedImage2DData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this structure is 100% identical to the normal Image2D GPU structure -- I think it would make sense to combine them, and perhaps keep the enum so we can differentiate them if needed but keep the same code paths in evalMaterial.h?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair point I did miss when rebasing and simplifying the code.
Given what you mentioned, I'd go for completely removing the identical state and the matching enum. Would this work for you?
Both can be reintroduce later on if that's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd go for completely removing the identical state and the matching enum. Would this work for you?
Yes, works for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment addressed!
devices/rtx/scene/surface/material/sampler/CompressedImage2D.cpp
Outdated
Show resolved
Hide resolved
Both are having the exact same layout and semantic from a GPU perspective, there is no need to separate them.
|
LGTM, thanks! |
VisRTX supported compressed textures are BC1,2,3,4,5.
=> BC6,7 are not supported nor really tested.
Those file format would require implementing some vertical flip algorithm so texture are correctly oriented, which is lower priority for now.
TSD supports loading of a subset of DDS compressed image formats (BC1-7). Raw/non-block compressed textures are not supported by the reader.
Here's an avocado, compressed using BC3 compressed textures

Left handside is VisRTX, right handside is VisGL.