Draw grid lines and labels for inverted axes with INCREMENT_BY_VAL - #137
Merged
Conversation
With inverted boundaries (min > max), Region.ratio is negative, so XYStepCalculator produced a negative pixel step for INCREMENT_BY_VAL and INCREMENT_BY_FIT. XYGraphWidget.drawGrid walks the axis in pixel order, so its loop bounds came out reversed and no lines or labels were drawn. Keep the pixel step positive and let the value step carry the sign, which is what the other step modes already produce for inverted bounds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #137 +/- ##
============================================
+ Coverage 70.98% 71.02% +0.03%
- Complexity 1242 1244 +2
============================================
Files 109 109
Lines 5191 5194 +3
Branches 543 544 +1
============================================
+ Hits 3685 3689 +4
+ Misses 1151 1150 -1
Partials 355 355 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Fixes #125
Cause
With inverted boundaries, e.g.
setDomainBoundaries(10, 0, FIXED), the axis region has min greater than max, soRegion.ratiois negative. InXYStepCalculator, theINCREMENT_BY_VALandINCREMENT_BY_FITmodes derive the pixel step by dividing the value step by that ratio, giving a negative pixel step.XYGraphWidget.drawGridwalks each axis in pixel order and computes its loop bounds from the pixel step, so with a negative step the bounds came out reversed and the loop never ran: no grid lines, no labels. The other modes derive the pixel step from pixel lengths, so they stayed positive and worked, which is why the reporter only saw it withINCREMENT_BY_VAL.Fix
In
XYStepCalculator, when the computed pixel step is negative, negate both it and the value step. The pixel step is then always positive and the value step carries the axis direction, which is exactly whatINCREMENT_BY_PIXELSandSUBDIVIDEalready produce for inverted bounds.drawGridneeds no changes: it already computes each line's value as origin plus index times the signed value step.This is a narrower change than the reporter's proposed swap of the loop endpoints inside
drawGrid, and it fixes the inconsistency at its source so any other consumer ofStepsees the same convention across modes.Verification
StepCalculatorTest: new case with inverted real bounds assertsINCREMENT_BY_VALyields pixel step +1 and value step -1, and thatINCREMENT_BY_PIXELSandSUBDIVIDEproduce the same signs. Fails on the old code with pixel step -1.XYGraphWidgetTest: new case sets inverted domain and range bounds withINCREMENT_BY_VALand captures every grid line drawn. Asserts 101 lines per axis, that the leftmost domain line carries the value 100 and the rightmost 0, and that the top range line carries 0 and the bottom 100. Fails on the old code with zero lines drawn.