-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add translateY style parameter to legend + test #568
Conversation
139c4b0
to
6e7f5d4
Compare
src/chart/rendering/drawlegend.cpp
Outdated
|
||
canvas.setBrushGradient( | ||
{transform(line.begin), transform(line.end)}, | ||
gradient); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this setter is a bit overcomplicated for the problem.
What we would like to be able to do is to set a transparency gradient for the label, if needed.
So why not simply give a Math::SegmentedFunction<double> transparencyGradient, Geom::Line gradientPosition
parameters for DrawLabel
and calculate the color gradient in DrawLabel from the calculated text color?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it's not just the label but the marker needs to. And that will be a duplication for the same gradient problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It's unfortunate. Still could be more clean. I see three problems:
-
This class encapsulates three things (building of the ColorGradient, transforming the line, and setting it to the canvas). I think it would be better if it would have a singular purpose, building the ColorGradient.
-
(and 3.) How we build the color gradient is that we have an alpha/transparency gradient, and we get the color from outside. The class does not represent this, since it contains a colorgradient, and the transparency gradient is hard coded in the function.
I think a clearer and more generic approach would be to have a class, which stores an arbitrary alpha gradient and has a method which accepts a color and builds the ColorGradient.
Something like this:
/// legend draw:
fadeoutLine = markerWindowRect.leftSide()
fadeoutGradient = AlphaGradient({
{0.0, 0.0},
{fadeElementPercent, 1.0},
{1.0 - fadeElementPercent, 1.0},
{1.0, 0.0}
})
// marker draw and label draw
canvas.setBrushGradient(transform(fadeoutLine), fadeoutGradient.toColorGradient(color))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, draw label is a more generic lower level utility class then legend draw. I think it's not a good practice to make the interface of the lower level class more cryptic to optimise code duplication in the higher level user class.
No description provided.