Skip to content

ImGuiIntegration: add support for draw list user callbacks#98

Merged
mosra merged 3 commits intomosra:masterfrom
pezcode:imgui-drawlist-callback
Jul 28, 2022
Merged

ImGuiIntegration: add support for draw list user callbacks#98
mosra merged 3 commits intomosra:masterfrom
pezcode:imgui-drawlist-callback

Conversation

@pezcode
Copy link
Copy Markdown
Contributor

@pezcode pezcode commented Jul 28, 2022

Continuation of #84 with tests

(and an unrelated fix for a failing test in ImGui 1.88 👀)

Closes #84.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 28, 2022

Codecov Report

Merging #98 (f372b00) into master (fa60304) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   77.73%   77.82%   +0.09%     
==========================================
  Files          21       21              
  Lines         952      956       +4     
==========================================
+ Hits          740      744       +4     
  Misses        212      212              
Impacted Files Coverage Δ
src/Magnum/ImGuiIntegration/Context.cpp 98.63% <100.00%> (+0.03%) ⬆️

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 fa60304...f372b00. Read the comment docs.

@mosra mosra added this to the 2022.0a milestone Jul 28, 2022
@mosra mosra merged commit f372b00 into mosra:master Jul 28, 2022
@mosra
Copy link
Copy Markdown
Owner

mosra commented Jul 28, 2022

Amazing, I got nothing to complain about. Feels strange :D

Tangential: seeing the workaround for ImGui < 1.80, would it make sense to again bump the ImGui version that's used on the CI? The 1.72 is probably quite ancient nowadays. I think we discussed this before, but don't remember what the conclusion was.

@pezcode
Copy link
Copy Markdown
Contributor Author

pezcode commented Jul 28, 2022

Thanks for the speedy review 😌

would it make sense to again bump the ImGui version that's used on the CI? The 1.72 is probably quite ancient nowadays. I think we discussed this before, but don't remember what the conclusion was.

A bump would make sense, yeah. If you don't want to spend CI minutes on two versions, the best one would be what the majority of people are likely using. 1.83 is about a year old, but might be too fresh for your taste 😄
The good part about changes in this part of the repo is that you can change ImGui versions and run tests in a matter of 4 seconds. So keeping backwards compatibility just requires some minor due diligence on new PRs.

@mosra
Copy link
Copy Markdown
Owner

mosra commented Jul 28, 2022

Wasn't there some policy that if an ImGui version gets older than 2 years, it becomes "obsolete"? That's what I would aim for, together with regularly cleaning up code needed only for older versions... like we do with Assimp, for example.

Looking through the releases, 1.78 is from August 2020, so 1.77 would be the oldest one, 1.80 is still some months away :) There are a few projects, demos and examples that don't use ImGui that heavily so they have whatever version was current at the time and don't upgrade ever again. I don't want to break these "just because" but I also don't want to have extra maintenance overhead keeping ancient versions working. I'll bump to 1.77 then, and next time there are additions to this library, we can bump & cleanup again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants