Skip to content

fix: use correct RGB shifts on big endian#1830

Open
zeldin wants to merge 1 commit into
hoffstadt:masterfrom
zeldin:endian-fix
Open

fix: use correct RGB shifts on big endian#1830
zeldin wants to merge 1 commit into
hoffstadt:masterfrom
zeldin:endian-fix

Conversation

@zeldin

@zeldin zeldin commented Jul 4, 2022

Copy link
Copy Markdown

name: Pull Request
about: Create a pull request to help us improve
title: ''
assignees: ''


Description:

This fixes incorrect colors in DearPyGui when running on big-endian systems. Since e.g. GL_RGBA means that R is in the "first" bits, this means the least significant bits on little-endian and the most significant bits on big-endian, so the shifts need to be different.

Tested on ppc64 (big endian) and aarch64 (little endian).

Concerning Areas:

@zeldin zeldin requested a review from hoffstadt as a code owner July 4, 2022 20:33
@Pcothren

Pcothren commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

@zeldin

zeldin commented Jul 5, 2022

Copy link
Copy Markdown
Author

@Pcothren Do you mean setting the shifts in imconfig.h instead? It seems more logical to me to fix the broken defaults instead of overriding them, but either way works I guess. Or are you talking about a refactoring to remove all the IM_COL32 stuff from imgui.h completely?

@Pcothren

Pcothren commented Oct 1, 2022

Copy link
Copy Markdown
Contributor

Any changes in imgui.h would be overwritten when we pull in the newer version of imgui.h

@zeldin

zeldin commented Oct 1, 2022

Copy link
Copy Markdown
Author

Well, wouldn't the correct way to deal with that be to upstream the bugfix to whereever you are pulling new versions from?
If you were thinking about overriding the defines in the IMGUI_USER_CONFIG file, it doesn't seem like it would work with the imgui.h you have now since the defines are unconditionally defined (and used in inline functions) after including the user config file. So you'd need a change in imgui.h anyway...

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