New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 872586 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Aug 21
Cc:
EstimatedDays: ----
NextAction: 2018-08-27
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

8.9% regression in rendering.mobile at 578558:578764

Project Member Reported by toyoshim@chromium.org, Aug 9

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=872586

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=d0e863cf067a8859c92525e9689e64c4938e403b10e5afeddf0173bedbe1be52


Bot(s) for this bug's original alert(s):

android-nexus5x-perf
Cc: danakj@chromium.org sky@chromium.org trchen@chromium.org
Owner: sky@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 3 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/16a1b48c640000

Reland: [Blink] Unify replaced element clipping by trchen@chromium.org
https://chromium.googlesource.com/chromium/src/+/112f71fce5ea31c1a4a92a2161a2d3ba77b20b35
0.9159 → 1.006 (+0.08977)

Move some of VisualProperties to RenderWidget::Init(). by danakj@chromium.org
https://chromium.googlesource.com/chromium/src/+/628bdce65237022927e4694d55cb4df8fd0af297
0.9963 → 1.347 (+0.3505)

Revert "Move some of VisualProperties to RenderWidget::Init()." by sky@chromium.org
https://chromium.googlesource.com/chromium/src/+/ae08f33b34db0a95b188315f64f6bda747e77351
1.308 → 0.9908 (-0.3176)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Owner: trchen@chromium.org
I'm guessing this is trchen, as the last two cancel themselves out.
Yes I confirm it is my CL, and the regression is due to behavior change in clip anti-aliasing.
Here is a minimal repro: http://output.jsbin.com/xekacayino/quiet

Before my CL, this content rect clip for non-composited <canvas> has disabled anti-aliasing, as opposed to almost everything else in Blink. My CL changed it to use the generic code path based on clip property node, and we only use anti-aliased clip.

Note that this has nothing to do with sub-pixel layout, so for example:
<canvas style="position:relative; left:0.5px;"></canvas>
The clip will still be snapped to integer CSS pixels.

It only changes behavior if the clip is not aligned to physical pixels (after mapped by raster scale and stuff). Nexus 5X is unfortunate because it has a terribly non-smooth device scale factor 2.6 .

In conclusion, this regression only happens if all criteria below are met:
* Non-composited <canvas> (Intrinsic size smaller than 256x256?)
* With non-composited transform that is not pixel-aligned. (Including fractional raster scale)

My personal opinion is that this regression is acceptable, because canvas that need overflow clip are rare comparing to regular elements that need overflow clip. If antialised-clip is something we don't want to pay on high-DPI mobile devices, we probably should disable it on all overflow clip.
Cc: chrishtr@google.com fs...@chromium.org
NextAction: 2018-08-27
Adding fserb@ from canvas team and chrishtr@ from paint team, who may have opinion on whether we can accept this.

Will close as WontFix in a week if no objection was made.
I agree with the reasoning in comment 5, but would like to hear from the Canvas team
also.
Status: WontFix (was: Assigned)
yeah, this makes sense to me too.
The NextAction date has arrived: 2018-08-27

Sign in to add a comment