Fix slider sub/add-page rendering#345
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
- Coverage 84.96% 84.92% -0.05%
==========================================
Files 49 49
Lines 3912 3914 +2
==========================================
Hits 3324 3324
- Misses 588 590 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Actually, I somehow didnt notice that extra handle... 🤔 |
Czaki
left a comment
There was a problem hiding this comment.
Looking on implementation, it looks correct.
What code did you use for testing?
tlambert03
left a comment
There was a problem hiding this comment.
thanks @brisvag
i'm happy with this too. I had claude make a big contact sheet of all kinds of before/after cases, and in almost every case it looks equal/better:
the only vaguely interesting case is the plain, unstyled QDoubleSlider, on macos:
it looks like the add-page (unfilled area to the right of the slider) is not getting the native style anymore. It it subtle, but it is noticeable:
so, i think we're probably paying a small cost here (losing nativeness) for the case where people are using stylesheets.
claude suggestion for a "cleaner fix"
Upstream QSlider::paintEvent (qtbase qslider.cpp ≈ lines 245–256) draws the slider in one pass with SC_SliderGroove | SC_SliderHandle (+ ticks). The two-pass split in _generic_slider.py:299-330 is the actual deviation. Collapsing to one combined call would:
- still fix the QSS ::sub-page/::add-page case (same code path as the PR),
- avoid the shared-NSSlider state pollution and likely restore the native blue accent,
- match what Qt itself does.
The reason superqt splits into two passes today is the interleaved manual tick-drawing for the Monterey QSS hack (paintEvent lines 309–328). That manual tick pass can run after a single combined drawComplexControl — it draws on top either way.
but i haven't checked to see A) how much work that is and B) whether it retains the native look, while still getting the fixes for the styled case
Just my eyes 🫠 copilot threw some tests together but as usual they are kinda useless and hard to parse, so I just tried myself. Luckily @tlambert03 came in with the big comparison :P
With my limited understanding, it seems a bit tricky, because RangeSlider inherits from the generic slider code and AFAICT it's necessary to have the split in there? I played around with that part of thhe code but I'm lost :P |

This was a surprisingly small fix, that I could have never found out by myself, but codex found super quickly :P
Both problems from the issue are solved!
Before:
After: