666097 - [Azure] Radial gradients in canvas don't always work as expected

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

In Azure canvas radial gradients will behave incorrectly if the inner circle intersects with, or lies outside of the outer circle.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Fix Azure radial gradient behavior.

This patch makes Azure use special shaders to create radial gradients.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Fix Azure radial gradient behavior v2

This fixes some minor bugs.

Comment 3

[Bas Schouten (:bas.schouten)]

Attachment: Fix Azure radial gradient behavior v3

Fix another small bug.

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Properly report passing

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: Fix Azure radial gradient behavior v3

Upload the right patch

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 544588 [details] [diff] [review]
Properly report passing

Doesn't this need changes for the new tests that pass?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 544589 [details] [diff] [review]
Fix Azure radial gradient behavior v3

Review of attachment 544589 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetD2D.cpp
@@ +1408,5 @@
> +    rawStops.resize(stops->mStopCollection->GetGradientStopCount());
> +    stops->mStopCollection->GetGradientStops(&rawStops.front(), rawStops.size());
> +
> +    RefPtr<ID3D10Texture1D> tex = CreateGradientTexture(rawStops);
> +

I'd move the stops getting into the CreateGradientTexture fucntion.

@@ +1445,5 @@
> +
> +    mPrivateData->mEffect->GetVariableByName("DeviceSpaceToUserSpace")->
> +      AsMatrix()->SetMatrix(matrix);
> +
> +    float A = pow(dc.x, 2) + pow(dc.y, 2) - pow(dr, 2);

I don't know if you care but VS2008 (didn't check 2010) doesn't turn pow(x, 2) into x*x

@@ +1454,5 @@
> +      mPrivateData->mEffect->GetVariableByName("A")->AsScalar()->SetFloat(A);
> +      mPrivateData->mEffect->GetTechniqueByName("SampleRadialGradient")->
> +        GetPassByIndex(0)->Apply(0);
> +    }
> +  }

Can this hunk be a separate function?

::: gfx/2d/HelpersD2D.h
@@ +172,5 @@
> +  }
> +
> +  return false;
> +}
> +

Can we come up with a better name for IsComplexPattern()?

::: gfx/2d/ShadersD2D.fx
@@ +126,5 @@
> +    Output.PixelCoord.y = ((1.0f - Output.Position.y) / 2.0f) * dimensions.y;
> +    Output.PixelCoord.xy = mul(float3(Output.PixelCoord.x, Output.PixelCoord.y, 1.0f), DeviceSpaceToUserSpace).xy;
> +    return Output;
> +}
> +

Can you add some comments about what this shader is doing.

@@ +186,5 @@
> +    }
> +
> +    return tex.Sample(sSampler, float2(t, 0.5)) * mask.Sample(sMaskSampler, In.MaskTexCoord).a;
> +};
> +

Some more documentation here wouldn't hurt. It's also worth saying something like: for a more detailed derivation see pixman.

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 544589 [details] [diff] [review]
Fix Azure radial gradient behavior v3

This patch is one of the fixes for Azure we need to take in the early mozilla-aurora space.

Comment 9

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Fix azure radial gradient behavior v4

Updated with review comments. Carrying r+.

Comment 10

[Joe Drew (not getting mail)]

I folded these two patches together and pushed them to inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/c0eaec585ea7

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/c0eaec585ea7

Comment 12

[Bas Schouten (:bas.schouten)]

Keeping this open for aurora approval!

Comment 13

[Bas Schouten (:bas.schouten)]

Pushed:

http://hg.mozilla.org/releases/mozilla-aurora/rev/ce54da315f7a

Sadly with the wrong bug number in the comment :(.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

The condition returned from IsPatternSupportedByD2D is reversed.

Comment 15

[AndreiD[QA]]

Build:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0

Setting this bug as verified fixed for firefox 7. The fix that landed on Aurora is visible in the Beta repository (i.e. http://hg.mozilla.org/releases/mozilla-beta/file/51f4341b6871/content/canvas/src/nsCanvasRenderingContext2DAzure.cpp). Also there are no failures with the attached tests to check the radial gradient (i.e. test_2d.gradient.radial.cone.top.html)