1851963 - Color Gradient does not work as expected with hardware acceleration enabled. [RTX A500, canvas2d, d2d]

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[hanichkranich]

Attachment: 2023-09-07 08_59_43-Canvas Drawing — Mozilla Firefox.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/116.0.0.0 Safari/537.36 Edg/116.0.1938.62

Steps to reproduce:

Opened https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/createConicGradient and checked the "Rectangle result" image.

Hardware acceleration is enabled, RTX A500.

Without hardware acceleration it works as expected.

Actual results:

The image is shown but with wrong and faded colours. (see image)

Expected results:

The colours should be normal.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: CanvasWebGL' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

Can repro on Windows11x64 with D2d-Canvas.

Regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=38b06f0c751021f94bebb032242b2326666ec476&tochange=b2154b397ef787592eb1b88ebade9957d33df816
Regressing bug (from mozregression) : Bug 1834815 - Enable full+cross LTO by default for shippable Linux and Windows build r=glandium

I cant repro with Skia-canvas. So it looks like Bug 1834815 broke D2D canvas backend somehow??

Comment 3

[Mayank Bansal]

Attachment: standalone_Testcase.html

Comment 4

[Mayank Bansal]

Attachment: about:support

Comment 5

[Mayank Bansal]

Attachment: d2d-Canvas Vs Skia-canvas Vs Chrome.png

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1834815

:sergesanspaille, since you are the author of the regressor, bug 1834815, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 7

[[:sergesanspaille]]

From a distance, this may be a compiler backend issue... But I'm not quite sure about the approach that should be taken to narrow this :-/

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1834815

Comment 10

[[:sergesanspaille]]

Just to confirm that it's related to Full LTO, can someone compiled a version linked with either ThinLTO or no lto at all and confirm the bug no longer appears?

Comment 13

[Lee Salzman [:lsalzman]]

I have no idea why LTO would cause this, other than it maybe messing up something in DrawTargetD2D1, like dispatching to the wrong shader somehow?

Jeff, any ideas?

Comment 15

[Jeff Muizelaar [:jrmuizel]]

Maybe some undefined behaviour in this function: https://searchfox.org/mozilla-central/rev/97feebcab27f1a92e70ceacaa77211e9eaba0e6e/gfx/2d/ConicGradientEffectD2D1.cpp#308?

Comment 16

[Lee Salzman [:lsalzman]]

Attachment: Bug 1851963 - Use unsigned char in ConicGradientEffectD2D1. r?jrmuizel

Although char is usually unsigned, somehow the conversion between float to
char may be clamping, as noted in bug 1787085. We want to explicitly use
unsigned char here, which is the right thing to do.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e41427b3c43b
Use unsigned char in ConicGradientEffectD2D1. r=jrmuizel

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e41427b3c43b

Comment 19

[Mayank Bansal]

The attached testcase is colored as expected on the latest Nightly.

Comment 20

[Lee Salzman [:lsalzman]]

Comment on attachment 9382092 [details]
Bug 1851963 - Use unsigned char in ConicGradientEffectD2D1. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Conic gradients are broken on Windows.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small fix to remove some undefined behavior that is causing undesired clamping of rendering results to be generated. We have employed a similar fix several releases ago for radial gradients that was well-tolerated.
• String changes made/needed:
• Is Android affected?: Unknown

Comment 21

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9382092 [details]
Bug 1851963 - Use unsigned char in ConicGradientEffectD2D1. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Conic gradients are broken on Windows.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small fix to remove some undefined behavior that is causing undesired clamping of rendering results to be generated. We have employed a similar fix several releases ago for radial gradients that was well-tolerated.
• String changes made/needed:
• Is Android affected?: No

Comment 22

[Dianna Smith [:diannaS]]

Comment on attachment 9382092 [details]
Bug 1851963 - Use unsigned char in ConicGradientEffectD2D1. r?jrmuizel

Approved for 124.0b4

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/a153cd3cd273

Comment 24

[Pascal Chevrel:pascalc]

Comment on attachment 9382092 [details]
Bug 1851963 - Use unsigned char in ConicGradientEffectD2D1. r?jrmuizel

Low risk, let's take it in our planned dot release next week, thanks.

Comment 25

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/691b4586eb7a

Comment 26

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-release/rev/691b4586eb7a