VOOZH about

URL: https://bugzilla.mozilla.org/2031507

⇱ 2031507 - Skia Lanczos3 downscaling: fix rounding bug


Closed Bug 2031507 Opened 2 months ago Closed 1 month ago

Skia Lanczos3 downscaling: fix rounding bug

Skia Lanczos3 downscaling: fix rounding bug
Core
Graphics
Firefox 149
Unspecified
Unspecified
defect
Points:
---
RESOLVED FIXED
RESOLVED
FIXED
152 Branch
Iteration:
---
a11y-review
Accessibility Severity
Performance Impact
Size Estimate
Webcompat Priority
Webcompat Score
Tracking Status
firefox152 --- fixed
Tracking Status
relnote-firefox
thunderbird_esr115
thunderbird_esr140
firefox-esr115
firefox-esr140
firefox-esr153
firefox152
firefox153
firefox154
---
QA Whiteboard:
[qa-triage-done-c153/b152]
Has STR:
---
Change Request:
---
Bug Flags:
Signature:
None
This bug is publicly visible.

 
Assignee

Description

2 months ago

Steps to reproduce:

The Skia Lanczos3 downscaler in gfx/2d/SkConvolver.cpp and its SIMD variants use arithmetic right shift when converting fixed-point accumulators to pixel values. This truncates the fractional part rather than rounding towards the nearest integer value.

This happens once during the horizontal pass, and once during the vertical pass. Each pass loses an average of 0.5 per channel, for a combined bias of roughly -1.0 average across both passes, darkening the output image.

The fix is to add the equivalent of +0.5 in fixed point to the number before truncation.

I benchmarked this with SSE2 and AVX2 backends, and I saw no difference. This should also apply to the NEON backend -- it's a single-cycle operation on all of these architectures.

Assignee: nobody → tle
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

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

Component: Untriaged → Graphics
Product: Firefox → Core
Attachment #9569556 - Attachment description: WIP: Bug 2031507 - Add rounding to SkConvolver fixed-point truncation to prevent image darkening. → Bug 2031507 - Add rounding to SkConvolver fixed-point truncation to prevent image darkening.
Assignee

Comment 3

2 months ago

A demo of this issue is here:
https://tourmaline-peony-ff2b1c.netlify.app/

It's a checkerboard image of (100, 100, 100) and (102, 102, 102) pixels.

Proper resizing should scale it to an image of (101, 101, 101) pixels, but current firefox generates a (100, 100, 100) image due to truncation instead of rounding.

Comment 4

1 month ago
Pushed by lsalzman@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/b324106f42b9 https://hg.mozilla.org/integration/autoland/rev/89a6edff943a Add rounding to SkConvolver fixed-point truncation to prevent image darkening. r=gfx-reviewers,lsalzman

Comment 5

1 month ago
Pushed by amarc@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/e74e67b3b94d https://hg.mozilla.org/integration/autoland/rev/5315122f6996 Revert "Bug 2031507 - Add rounding to SkConvolver fixed-point truncation to prevent image darkening. r=gfx-reviewers,lsalzman" for causing xpcshell failures @ test_imgtools

Comment 6

1 month ago

Backed out for causing xpcshell failures

Flags: needinfo?(tle)

Comment 7

1 month ago
Pushed by lsalzman@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/e2ff986477de https://hg.mozilla.org/integration/autoland/rev/8c0b7ff3ce73 Add rounding to SkConvolver fixed-point truncation to prevent image darkening. r=gfx-reviewers,lsalzman

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/59398 for changes under testing/web-platform/tests

Assignee

Comment 9

1 month ago

The xpcshell failure was fixed.

Flags: needinfo?(tle)

Comment 10

1 month ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 152 Branch
QA Whiteboard: [qa-triage-done-c153/b152]
You need to log in before you can comment on or make changes to this bug.