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

Columns: Make sure the ClipRect is valid. #3475

Closed
wants to merge 1 commit into from

Conversation

szreder
Copy link

@szreder szreder commented Sep 16, 2020

When a window with columns is dragged horizontally outside of the viewport, the ClipRect of a column may fall entirely outside ClipRect of the containing window. Intersecting those ClipRects using ClipWith() member function may cause the resulting rectangle to be invalid (meaning: ClipRect.Min.x > ClipRect.Max.x).

Fix this issue by using ClipWithFull() instead.

@ocornut
Copy link
Owner

ocornut commented Sep 16, 2020

Hello,
Thanks for the PR. I think the fix is fine as is for this specific case, however the way things are designed, an “inverted” rectangle should be perfectly valid as “null” rectangle.

So my question is: could you describe which issue you are getting which made you dig into this fix?

Thank you!

@szreder
Copy link
Author

szreder commented Sep 16, 2020

Hello,
the "inverted" rectangle gets passed to the ImDrawCmd for which I assumed (based on related IM_ASSERT()s sprinkled here and there) that its ClipRect need to be valid (non-inverted). My renderer balked at such inverted rectangles, which caused me to investigate the issue in the first place.

At first I though about hackfixing the renderer to accept such rectangles, but after diving into ImGui's code and noticing the aforementioned asserts, I came to the conclusion that the proper fix is to use ClipWithFull(). My conviction was further strenghtened after reading comments beside struct ImRect ClipWith() and ClipWithFull() member functions.

Best regards

@ocornut
Copy link
Owner

ocornut commented Sep 16, 2020

the "inverted" rectangle gets passed to the ImDrawCmd

I can't repro that in the unmodified demo, because high-level primitives are clipped, making the ImDrawCmd empty and culled before it gets a chance to reach the rendering back-end. However, I can see that adding e.g. unclipped ImDrawList calls create the situation you mention.

ImDrawList::PushClipRect() does the equivalent of ClipWithFull() so the culprit is that custom code calling the optimized shortcut SetWindowClipRectBeforeSetChannel() needs to pass a positive or null ClipRect.

I'll merge you fix and will make sure other users of SetWindowClipRectBeforeSetChannel() also perform clipping the expected way.

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

Successfully merging this pull request may close these issues.

2 participants