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

Fix color representation for texture in example #15

Merged
merged 1 commit into from
Jul 29, 2020
Merged

Fix color representation for texture in example #15

merged 1 commit into from
Jul 29, 2020

Conversation

atbentley
Copy link
Contributor

I'm a bit over my head here but it seems like dear imgui expects colours to come in linear rgb space, so when we need to load textures we need to override the deafault Repr which is set to Srgb.

Does this mean if I want to use a texture in rendy and in dear imgui I need to load it twice? Once with Srgb for rendy and once with Unorm for dear imgui. I think this memory inefficient but computationally fast, so it's probably the right choice for now. However it does seem like there are some open discussions and even a PR for allowing dear imgui to work directly with srgb, if that gets resolved then there might be a away to use a single texture handle for both rendy and dear imgui.

Before:

Screen Shot 2020-06-05 at 3 21 24 pm

After:

Screen Shot 2020-06-07 at 11 45 05 am

@vladbat00 vladbat00 requested a review from jaynus June 7, 2020 20:07
@vladbat00
Copy link
Member

Hey, thanks for the PR!

Regarding your concern:

Does this mean if I want to use a texture in rendy and in dear imgui I need to load it twice?

I don't think so, I guess you can just choose the representation format which is compatible with imgui - and that's all. I can't see any reason why another image handle would be needed there.

Though I'm by no means rendy/Vulkan expert, maybe we could summon @zakarumych to the discussion

@vladbat00
Copy link
Member

Well, as far as it works correctly (definitely better in this aspect than it was), I'm happy to merge it

@vladbat00 vladbat00 merged commit 18d1140 into amethyst:master Jul 29, 2020
@atbentley atbentley deleted the fix-demo-texture branch September 16, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants