Skip to content
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

Improvements to gradients. #16666

Merged
merged 7 commits into from May 7, 2017

Duplicate first gradient stop if necessary.

If the first stop of a non-repeating gradient is
not placed at offset 0.0 it is duplicated at this
position. This solves the problem of the first
stop being ignored if it is placed at the same
offset as the next stop.
  • Loading branch information
pyfisch committed May 6, 2017
commit 72db8d85553c3d760ceed382f3a63e7287d042f7
@@ -650,8 +650,8 @@ fn convert_gradient_stops(gradient_items: &[GradientItem],
}

// Step 3: Evenly space stops without position.
// Note: Remove the + 1 if fix_gradient_stops is changed.
let mut stops = Vec::with_capacity(stop_items.len() + 1);
// Note: Remove the + 2 if fix_gradient_stops is changed.
let mut stops = Vec::with_capacity(stop_items.len() + 2);
let mut stop_run = None;
for (i, stop) in stop_items.iter().enumerate() {
let offset = match stop.position {
@@ -698,15 +698,25 @@ fn convert_gradient_stops(gradient_items: &[GradientItem],
}

#[inline]
/// Duplicate the last stop if its position is smaller 100%.
/// Duplicate the first and last stops if necessary.
///
/// Explanation by pyfisch:
/// If the last stop is at the same position as the previous stop the
/// last color is ignored by webrender. This differs from the spec
/// (I think so). The implementations of Chrome and Firefox seem
/// to have the same problem but work fine if the position of the last
/// stop is smaller than 100%. (Otherwise they ignore the last stop.)
///
/// Similarly the first stop is duplicated if it is not placed
/// at the start of the virtual gradient ray.

This comment has been minimized.

@emilio

emilio May 6, 2017

Member

Thanks for the comment, but... I guess the question is, what does the spec say? :)

This comment has been minimized.

@pyfisch

pyfisch May 7, 2017

Author Contributor

If multiple color stops have the same position, they produce an infinitesimal transition from the one specified first in the rule to the one specified last. In effect, the color suddenly changes at that position rather than smoothly transitioning.

Well this is what the spec says about color stops with the same position. There are no special rules for the first and last stop. The way I understand the spec testcase 12 should have a green background with the circle in the middle like example 11 but bigger. Unfortunately this is not what is implemented in Blink and Gecko. Maybe there is a note in the spec but I missed it completely. 😕

servo/webrender#1189 seems to do something similiar but on the webrender side. (I have not tested if it actually works like this function.

This comment has been minimized.

@emilio

emilio May 7, 2017

Member

Yeah, I remember to have seen that PR, thought it'd landed. We can land this for now and then remove/modify fix_gradient_stops if necessary. Thanks for the explanation!

fn fix_gradient_stops(stops: &mut Vec<GradientStop>) {
if stops.first().unwrap().offset > 0.0 {
let color = stops.first().unwrap().color;
stops.insert(0, GradientStop {
offset: 0.0,
color: color,
})
}
if stops.last().unwrap().offset < 1.0 {
let color = stops.last().unwrap().color;
stops.push(GradientStop {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.