Add additional short lived gradient cache for border corners

RESOLVED FIXED in mozilla19

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

(Blocks: 1 bug)

unspecified
mozilla19
x86_64
Windows 8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Snappy])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 675585 [details] [diff] [review]
Add an additional cache for border corner gradients

Some workloads are showing very large times spent generating the gradients in order to render border corners. Considering a lot of designs will have several similar borders, and a lot of borders will have multiple corners using the same gradients, or a gradient exactly reversed, we could benefit considerably from a cache when using Azure content. This patch adds an Azure-specific drawing path for common border corners and caches gradients here.

This is only guaranteed to give correct (and in some cases more correct) rendering with the Direct2D Azure and Quartz Cairo backends. For other cairo backends it could create un-antialiased border corner transitions since that at least used to be pixman's behaviour for hard stops.

On my profiles this makes up to a 30% difference in painting for certain workloads.
Attachment #675585 - Flags: review?(jmuizelaar)
(Assignee)

Comment 1

5 years ago
Created attachment 675588 [details] [diff] [review]
Add an additional cache for border corner gradients v2

Fixed some whitespace.
Attachment #675585 - Attachment is obsolete: true
Attachment #675585 - Flags: review?(jmuizelaar)
Attachment #675588 - Flags: review?(joe)
Comment on attachment 675588 [details] [diff] [review]
Add an additional cache for border corner gradients v2

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

::: gfx/2d/Types.h
@@ +121,5 @@
>  
> +  uint32_t ToABGR() const
> +  {
> +    return uint32_t(r * 255.0f) | uint32_t(g * 255.0f) << 8 |
> +      uint32_t(b * 255.0f) << 16 | uint32_t(a * 255.0f);

What am I missing, since this seems like it's going to overwrite the r bits with a.

Also, indent correctly.

::: layout/base/nsCSSRenderingBorders.cpp
@@ +53,5 @@
> +    : mColor1(aColor1.ToABGR()), mColor2(aColor2.ToABGR())
> +    , mBackendType(aBackendType)
> +  { }
> +
> +  BorderGradientCacheKey(const BorderGradientCacheKey* aOther)

Did you mean this to be BorderGradientCacheKey& aOther? As is, this isn't a copy constructor (and it seems like this class would be fine with the default copy constructor).

@@ +1273,5 @@
> +  const twoFloats gradientCoeff[4] = { { -1, +1 },
> +                                       { -1, -1 },
> +                                       { +1, -1 },
> +                                       { +1, +1 } };
> +  

whitespace (which I can't help but notice you removed from the place you copied this from :) )

@@ +1535,5 @@
> +
> +  const twoFloats centerAdjusts[4] = { { 0, +0.5 },
> +                                        { -0.5, 0 },
> +                                        { 0, -0.5 },
> +                                        { +0.5, 0 } };

indentation is wrong here
Attachment #675588 - Flags: review?(joe) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Joe Drew (:JOEDREW! \o/) from comment #2)
> Comment on attachment 675588 [details] [diff] [review]
> ::: layout/base/nsCSSRenderingBorders.cpp
> @@ +53,5 @@
> > +    : mColor1(aColor1.ToABGR()), mColor2(aColor2.ToABGR())
> > +    , mBackendType(aBackendType)
> > +  { }
> > +
> > +  BorderGradientCacheKey(const BorderGradientCacheKey* aOther)
> 
> Did you mean this to be BorderGradientCacheKey& aOther? As is, this isn't a
> copy constructor (and it seems like this class would be fine with the
> default copy constructor).

Nope, this is what the templates need.
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/651c802473d5 - apparently it wasn't what something needed, since it doesn't build.
(Assignee)

Comment 5

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/ea35621ef12a
https://hg.mozilla.org/mozilla-central/rev/ea35621ef12a
https://hg.mozilla.org/mozilla-central/rev/08c38354ae7c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Whiteboard: [Snappy]

Updated

5 years ago
Blocks: 750871
You need to log in before you can comment on or make changes to this bug.