STY: Update macosx zoom rect styling #24367
Conversation
24dd283 to
50f25ed
Compare
src/_macosx.m
Outdated
| // in phase. | ||
| NSBezierPath *white_path = [NSBezierPath bezierPathWithRect: rubberband]; | ||
| NSBezierPath *black_path = [NSBezierPath bezierPathWithRect: rubberband]; | ||
| double dash_pattern[2] = {3, 3}; |
There was a problem hiding this comment.
Should be CGFloat technically.
| double dash_pattern[2] = {3, 3}; | |
| CGFloat dash_pattern[2] = {3, 3}; |
| @@ -1526,9 +1537,11 @@ - (void)otherMouseDragged:(NSEvent *)event { [self mouseDragged: event]; } | |||
|
|
|||
| - (void)setRubberband:(NSRect)rect | |||
| { | |||
| if (!NSIsEmptyRect(rubberband)) { [self setNeedsDisplayInRect: rubberband]; } | |||
| // The space we want to redraw is a union of the previous rubberband | |||
| // with the new rubberband and then expanded (negative inset) by one | |||
There was a problem hiding this comment.
Just checking if this is in logical space or physical space (i.e., should be one or one * display ratio)?
There was a problem hiding this comment.
On HiDPI, this still clears the region and I'm not sure we want to bring in the deviceScale/contextRegion if we don't absolutely need to here. The linewidth is drawn in points and centered on the rectangle area, so the default linewidth of 1 expands 0.5 points on either side of the rect. (linewidth section from here
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CocoaDrawingGuide/Paths/Paths.html#//apple_ref/doc/uid/TP40003290-CH206-BBCDEJGD)
|
Two things (I cannot test or judge the code...):
|
Match the zoom box styling of the other backends with alternating white/black paths.
50f25ed to
cb68ffd
Compare
This is already so fast I don't think it is worth worrying about performance. I also don't want to overlap if we can avoid it so we don't have any additional weird aliasing / blending going on for the overlapping regions.
I see this as an enhancement/improvement and not a bugfix. I really don't think these should be backported, but rather if we want it in sooner, a new minor release should happen sooner. The cursor styling and other aesthetic changes like that should be in minor versions not patch IMO. |
|
OK! Makes sense. I was under the impression that something had changed in recent versions, but maybe it is just a consequence of more and more people using dark themes that we get those bug reports now... |
|
ping for a macosx user to review this. It'd be nice to get in for the 3.7 release to keep the backends consistent in their styling. |
|
Confirmed that this looks good axes with white, black, and "0.5" patch colors. |
I think the lines should be the same thickness as before, just dashed now. It is easy enough to increase the thickness if desired too. On preview for me, the dash pattern actually rotates around. |
Yes, but I wasn't going to ask you to do that. However, that is what made me think there must be a selector API |
What do you mean by rotates around? |
|
The dash pattern precesses around the selection in a clockwise manner |
PR Summary
Match the zoom box styling of the other backends with alternating white/black paths.
closes #23777
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst