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

Introduce IMGUI_USE_BGRA_PACKED_COLOR in imconfig.h. #767

Closed
wants to merge 2 commits into from
Closed

Introduce IMGUI_USE_BGRA_PACKED_COLOR in imconfig.h. #767

wants to merge 2 commits into from

Conversation

thedmd
Copy link
Contributor

@thedmd thedmd commented Aug 5, 2016

PR adds ability to tell ImGui to generate packed color in BGRA format instead of RGBA.
This change simplify backend implementation by removing need to convert colors in vertex buffer before uploading to engine. For ImGui packing colors differently make no difference and backend can use memcpy().

To change color packing uncomment #define IMGUI_USE_BGRA_PACKED_COLOR in imconfig.h

What changes. Hardcoded bit shifts were replaced by constants like IM_COL32_R_SHIFT. Depending of IMGUI_USE_BGRA_PACKED_COLOR being defined constants have different values.

@ratchetfreak
Copy link

Or you could change the vertex shader to do a color_out = color_in.bgra; instead of a color_out = color_in;

This will still allow use of memcpy in the backend.

@thedmd
Copy link
Contributor Author

thedmd commented Aug 5, 2016

@ratchetfreak In example application I can, but in engine I have no direct access to shaders. Yet, there may be no shaders at all and renderer may use fixed pipeline. This feature is handy in real world use when control over what you can change is limited.

@ocornut
Copy link
Owner

ocornut commented Aug 6, 2016

@thedmd Have you measure the cost of your RGBA>BGRA conversion loop at the end of the render? Is it actually meaningful?

Patch feedback

  • The macro IM_COL32_BLACK becomes wrong.
  • There's code in ImGui::Render that uses hardcoded hex values needs to be changed to use IM_COL32()
  • ValueColor() needs to be updated?

@thedmd
Copy link
Contributor Author

thedmd commented Aug 6, 2016

@ocornut No, I did not. I think it is very unlikely my conversion loop will get anywhere close to memcpy performance without committing many hours in utilization of processor extensions. But that is aftermath.
Real issue was not copying but format of packed color ImGui output. Engine I use is expecting different one and I cannot change that. If I can bend middle-ware to fit better I will try to do so to avoid having problem in the first place.

What is your opinion?

@ocornut
Copy link
Owner

ocornut commented Aug 6, 2016

it is very unlikely my conversion loop will get anywhere close to memcpy

My question is have you ever measured the cost of your naive conversion loop?
We are talking about ~10k mostly contiguous vertices which is a small amount. Even a naive conversion shouldn't take much time.

@thedmd
Copy link
Contributor Author

thedmd commented Aug 6, 2016

No, I didn't. Motivation was to avoid need of doing conversion, not to speed up copying.

When IMGUI_USE_BGRA_PACKED_COLOR is defined packed color hold in ImU32 use BGRA format instead RGBA.
@thedmd
Copy link
Contributor Author

thedmd commented Sep 26, 2016

PR was recreated due to lost references after changing fork origin. That was not my intention, sorry.
Updated PR.

@thedmd thedmd closed this Sep 26, 2016
ocornut added a commit that referenced this pull request Jun 5, 2018
…ible items for scoring (note that this only work assuming the NavFlattened child window has interactive items). Fixes accidentally hoping into a NavFlattened child. (#767)
ocornut added a commit that referenced this pull request Nov 8, 2021
actondev pushed a commit to actondev/imgui that referenced this pull request Nov 26, 2021
ocornut pushed a commit that referenced this pull request Apr 13, 2022
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.

3 participants