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

ImGuiIntegration: use provided index offset #88

Closed
wants to merge 1 commit into from

Conversation

pezcode
Copy link
Contributor

@pezcode pezcode commented Jan 12, 2022

Closes #86

Manually calculating the index buffer offset isn't correct (despite the comment for IdxOffset indicating otherwise). This surfaced with changes in v1.86 under certain circumstances, e.g. stacked modals. See ocornut/imgui#4845.
@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #88 (b471f2c) into master (da3c1ab) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
- Coverage   77.25%   77.20%   -0.05%     
==========================================
  Files          21       21              
  Lines         941      939       -2     
==========================================
- Hits          727      725       -2     
  Misses        214      214              
Impacted Files Coverage Δ
src/Magnum/ImGuiIntegration/Context.cpp 90.13% <100.00%> (-0.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da3c1ab...b471f2c. Read the comment docs.

@jvannugteren
Copy link

Thanks. This fix solves the issues in my usecase.

@pezcode
Copy link
Contributor Author

pezcode commented Jan 19, 2022

Same caveat as here: #90 (comment)

@mosra mosra added this to the 2020.0b milestone Jan 31, 2022
@mosra mosra added this to TODO in GUI via automation Jan 31, 2022
@mosra
Copy link
Owner

mosra commented Jan 31, 2022

Thank you! Merged as 3e7b175.

For the record, we agreed on not supporting ImGui releases older than 2 years, so it's fine this change raises the minimum requirement to 1.71.

@mosra mosra closed this Jan 31, 2022
GUI automation moved this from TODO to Done Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
GUI
  
Done
Development

Successfully merging this pull request may close these issues.

ImGui IdxOffset not honored correctly during frame drawing
3 participants