The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in Firefox 7

Status

()

Core
Graphics
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

(Depends on: 1 bug)

unspecified
mozilla8
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox7+ fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

6 years ago
In Azure canvas radial gradients will behave incorrectly if the inner circle intersects with, or lies outside of the outer circle.
(Assignee)

Comment 1

6 years ago
Created attachment 544575 [details] [diff] [review]
Fix Azure radial gradient behavior.

This patch makes Azure use special shaders to create radial gradients.
Attachment #544575 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

6 years ago
Created attachment 544579 [details] [diff] [review]
Fix Azure radial gradient behavior v2

This fixes some minor bugs.
Attachment #544575 - Attachment is obsolete: true
Attachment #544579 - Flags: review?(jmuizelaar)
Attachment #544575 - Flags: review?(jmuizelaar)
(Assignee)

Comment 3

6 years ago
Created attachment 544582 [details] [diff] [review]
Fix Azure radial gradient behavior v3

Fix another small bug.
Attachment #544579 - Attachment is obsolete: true
Attachment #544582 - Flags: review?(jmuizelaar)
Attachment #544579 - Flags: review?(jmuizelaar)
(Assignee)

Comment 4

6 years ago
Created attachment 544588 [details] [diff] [review]
Properly report passing
Attachment #544588 - Flags: review?(jmuizelaar)
(Assignee)

Comment 5

6 years ago
Created attachment 544589 [details] [diff] [review]
Fix Azure radial gradient behavior v3

Upload the right patch
Attachment #544582 - Attachment is obsolete: true
Attachment #544589 - Flags: review?(jmuizelaar)
Attachment #544582 - Flags: review?(jmuizelaar)
Comment on attachment 544588 [details] [diff] [review]
Properly report passing

Doesn't this need changes for the new tests that pass?
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.
Attachment #544589 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 8

6 years ago
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.
Attachment #544589 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

6 years ago
Attachment #544588 - Flags: approval-mozilla-aurora?
Attachment #544588 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 9

6 years ago
Created attachment 544876 [details] [diff] [review]
Part 1: Fix azure radial gradient behavior v4

Updated with review comments. Carrying r+.
Attachment #544589 - Attachment is obsolete: true
Attachment #544876 - Flags: review+
Attachment #544876 - Flags: approval-mozilla-aurora?
Attachment #544589 - Flags: approval-mozilla-aurora?
I folded these two patches together and pushed them to inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/c0eaec585ea7
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/c0eaec585ea7
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
(Assignee)

Comment 12

6 years ago
Keeping this open for aurora approval!
Status: RESOLVED → REOPENED
tracking-firefox7: --- → ?
Resolution: FIXED → ---

Updated

6 years ago
Attachment #544588 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
Attachment #544876 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 13

6 years ago
Pushed:

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

Sadly with the wrong bug number in the comment :(.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
status-firefox7: --- → fixed
Resolution: --- → FIXED

Updated

6 years ago
tracking-firefox7: ? → +
Depends on: 672646
Whiteboard: [inbound]
The condition returned from IsPatternSupportedByD2D is reversed.

Comment 15

6 years ago
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)
Status: RESOLVED → VERIFIED

Updated

2 years ago
Depends on: 1164912
You need to log in before you can comment on or make changes to this bug.