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 overflow when using 32-bit ImDrawIdx #88

Merged
merged 1 commit into from
Aug 16, 2020

Conversation

bear24rw
Copy link
Contributor

For backends that don't support VtxOffset (the imgui emscripten example) we are forced to use a 32bit index buffer instead. This patch fixes an issue from #41 where the calculation of the maximum index would overflow. The benchmark plot now renders correctly in the emscripten example when compiling with -DImDrawIdx="unsigned int".

@bear24rw bear24rw force-pushed the imdrawidx_32bit branch 3 times, most recently from d358c17 to 2027119 Compare August 13, 2020 18:04
@epezent
Copy link
Owner

epezent commented Aug 13, 2020

Thanks for the fix! Can you make max_idx either static or a compile time constant so it's not reevaluated each time.

@bear24rw
Copy link
Contributor Author

Good idea, I just force pushed that change.

@epezent epezent merged commit c09d160 into epezent:master Aug 16, 2020
@epezent
Copy link
Owner

epezent commented Aug 20, 2020

@bear24rw , I don't know if you noticed, but I ended up changing your PR slightly. I replaced your ImPow calculation with some template code:

template <typename T>
struct MaxIdx { static const unsigned int Value; };
template <> const unsigned int MaxIdx<unsigned short>::Value = 65535;
template <> const unsigned int MaxIdx<unsigned int>::Value   = 4294967295;
...
unsigned int cnt = ImMin(prims, (MaxIdx<ImDrawIdx>::Value - DrawList._VtxCurrentIdx) / Renderer::VtxConsumed);
...

Does this still work as expected for you?

@bear24rw
Copy link
Contributor Author

Does this still work as expected for you?

Yes!

Ben1138 pushed a commit to Ben1138/implot that referenced this pull request Oct 2, 2024
Fix overflow when using 32-bit ImDrawIdx
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