If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

radial_compute_color (-moz-radial-gradient) is slow on ARM

NEW
Unassigned

Status

()

Core
Graphics
7 years ago
2 years ago

People

(Reporter: mbrubeck, Unassigned)

Tracking

(Blocks: 2 bugs, {helpwanted, mobile, perf})

Trunk
ARM
All
helpwanted, mobile, perf
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ARM-opt], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
http://html5test.com/ is very slow to render during panning, in Fennec on Android.  It takes several seconds to render each new region.

According to Chris Jones, pixman's radial_compute_color was the top culprit when he profiled with this page.  It would be nice if we had a faster version of this for ARM.
(Reporter)

Updated

7 years ago
Blocks: 527728

Comment 1

7 years ago
Yes, radial gradients is one of worse performing parts of pixman at the moment, and not only on ARM. The fact that their behaviour was recently changed (to match PDF specification [1]) and the absence of automatic tests is not helping much.

The biggest problem is the use of double precision floating point calculations. On ARM Cortex-A8 these are not pipelined and extremely slow. Single precision calculations are also slow with C code, but they can be optimized using NEON. And single precision should be enough in many or even most practical cases, but identifying the cases where lower precision would be sufficient is still a gray area.

There are two possible approaches:
a) Try to convert radial gradients code to single precision and NEON, with a hope that it would work correctly enough
b) Try to add some automated tests evaluating differences between an optimized implementation and a reference full precision implementation, implement some heuristic switching logic between low and high precision at runtime depending on gradient properties, finally optimize low precision implementation using NEON and verifying it for conformance with these automated tests.

With some luck (a) may be a lot faster to get up and running than (b). But if it does not work good enough right from the start, then it may actually require more efforts in the long run for debugging and fixing bugs and inconsistencies. And if later it gets decided that radial gradients behaviour needs to be changed again, then the whole work may need to be redone.

1. http://lists.freedesktop.org/archives/xorg-announce/2010-October/001441.html
Created attachment 542865 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM
Comment on attachment 542865 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM

The problem is that html5test has a radial background on the body element, and we end up in our horribly slow pixman path for the entire background! roc, how about something like this approach?
Attachment #542865 - Flags: review?(roc)
Another thing we could do (which Webkit does) is cache gradient background images.
Comment on attachment 542865 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM

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

::: layout/base/nsCSSRendering.cpp
@@ +2180,5 @@
>    gfxRect areaToFill =
>      nsLayoutUtils::RectToGfxRect(aFillArea, appUnitsPerPixel);
>    gfxMatrix ctm = ctx->CurrentMatrix();
>  
> +  if (!restrictTo.IsEmpty()) {

Instead of making an empty rect "special", I suggest we set restrictTo to the dirty rect up above, and here test whether restrictTo does not contain the dirty rect.
Comment on attachment 542865 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM

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

::: layout/base/nsCSSRendering.cpp
@@ +2180,5 @@
>    gfxRect areaToFill =
>      nsLayoutUtils::RectToGfxRect(aFillArea, appUnitsPerPixel);
>    gfxMatrix ctm = ctx->CurrentMatrix();
>  
> +  if (!restrictTo.IsEmpty()) {

In fact, how about moving the block that handles restrictTo up into the radial gradient code where we set restrictTo? We can hoist areaToFill up above that conditional, and 'dirty' too.
Assignee: nobody → ben
roc: review ping?
Can you address the comments?
Sorry, not sure how I missed that...
Created attachment 546619 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM

I realized that I can't just paint the entire background a solid color, because
the gradient may not be opaque. Review comments addressed too.
Attachment #546619 - Flags: review?(roc)
Attachment #542865 - Attachment is obsolete: true
Attachment #542865 - Flags: review?(roc)
Comment on attachment 546619 [details] [diff] [review]
radial_compute_color (-moz-radial-gradient) is slow on ARM

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

::: layout/base/nsCSSRendering.cpp
@@ +2147,5 @@
> +    // are completely disjoint since there is an alpha channel. Overpainting
> +    // will result in incorrect painting.
> +    gfxRect radialGfxRect = gfxRect(
> +      lineStart.x - outerRadius, lineStart.y - outerRadius,
> +      outerRadius * 2, outerRadius * 2) + areaToFill.TopLeft();

I think this is incorrect actually. aOneCellArea is repeated over the fill area. lineStart.x is relative to aOneCellArea (and all the cells, actually), not the fill area.

In fact I think you need to restructure your patch to handle tiling. And make sure that's tested...
Assignee: ben → doug.turner

Updated

6 years ago
Assignee: doug.turner → nobody
(Reporter)

Updated

6 years ago
Keywords: helpwanted
Whiteboard: [ARM-opt]
Attachment #546619 - Flags: review?(roc)
You need to log in before you can comment on or make changes to this bug.