[GUI] Color slider update and fixes#1997
Open
MrStevns wants to merge 18 commits into
Open
Conversation
- Slider is now bigger - Picker stays within the slider boundary - Slider now scales according to HDPI - Picker is now multi colored, rather than switching color - The middle of the picker now corresponds to the cursor position, rather than shifting closer to the nearest edge.
What I thought was caused by the device pixel ratio, ended up being me forgetting to compensate for the borderWidth.
Fix quantization issue where the hue slider skipped one value at the end. Because we already specify the color type, we don't need to specify the min and max from outside, so that has been removed as well
|
chchwy
added a commit
to chchwy/pencil2d
that referenced
this pull request
Apr 25, 2026
…ressions - Finding 1 (gradient endpoints): Confirmed PRE-EXISTING, not a PR regression - Finding 2 (division-by-zero): CRITICAL NEW BUG introduced by refactor pickerMaxDistance() lacks guard against zero/negative values - Other findings: Minor style/cleanup issues - Build system: Verified correct in qmake and CMake Blocker: Division-by-zero in colorPicked() must be fixed before merge.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Pencil2D’s color inspector UI rendering by modernizing the ColorSlider painting (bigger, clipped corners, HiDPI-aware, picker drawn inside bounds) and restoring a checkerboard-backed color preview via a dedicated widget, with shared slider drawing/geometry utilities added to core_lib.
Changes:
- Add reusable slider geometry + style drawing utilities in
core_lib/src/util. - Rework
ColorSliderrendering/caching and picker math (incl. HiDPI pixmap handling). - Replace the old palette-based preview with a new
ColorPreviewWidgetand update the color inspector layout.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| core_lib/src/util/slidergeometry.h | New slider geometry helper API. |
| core_lib/src/util/slidergeometry.cpp | Geometry helper implementations. |
| core_lib/src/util/drawsliderstyle.h | New slider style struct + drawing API. |
| core_lib/src/util/drawsliderstyle.cpp | Slider style drawing implementation. |
| core_lib/core_lib.pro | Registers new util sources/headers for qmake build. |
| app/ui/colorinspector.ui | Layout tweaks; swaps preview widget and rearranges controls. |
| app/src/colorslider.h | Uses new util helpers; adjusts slider API and cached state. |
| app/src/colorslider.cpp | New rendering/caching/picker logic + HiDPI pixmap changes. |
| app/src/colorpreviewwidget.h | New widget header for checkerboard-backed preview. |
| app/src/colorpreviewwidget.cpp | Preview widget painting implementation. |
| app/src/colorinspector.cpp | Wires new preview widget; updates slider init + HSV conversion logic. |
| app/app.pro | Registers new preview widget for qmake build. |
Comments suppressed due to low confidence (1)
app/src/colorslider.cpp:55
QPaintEvent::rect()can be a sub-rectangle during partial updates; creating the backing pixmap fromevent->rect().size()can produce a pixmap smaller than the widget, and thendrawPixmap(0, 0, ...)will leave the rest undrawn. Build the cache from the full widget size (e.g.this->size()/rect().size()) and use the paint event rect only for clipping if needed.
void ColorSlider::paintEvent(QPaintEvent* event)
{
drawColorBox(mColor, event->rect().size());
QPainter painter(this);
painter.drawPixmap(0, 0, mBoxPixmapSource);
painter.end();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MrStevns
force-pushed
the
color-slider-update-and-fixes
branch
from
3358772 to
898035a
Compare
Member
Author
|
All review comments have now been addressed |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slider appearance updates and other fixes.
Changes
Lately i've been rather intrigued by Data Oriented Design, so i'm also introducing some some slider painter utility namespaces. I think that this approach actually works better and is cleaner rather than creating painter classes, when the logic should be shared across components. What do you think?
Note: There is a known issue on macOS.. for some reason the spinboxes gets cut on the top and bottom as if something is squeezing it, however all the layout is set to be "preferred" so it shouldn't be possible. I haven't been able to fix this yet.