Port the CAIRO_COORD_MAX code in nsRenderingContext::FillRect to DrawTargetCairo::FillRect

RESOLVED FIXED in mozilla36

Status

()

Core
Graphics
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Currently we have some CAIRO_COORD_MAX handling code in nsRenderingContext:

void
nsRenderingContext::FillRect(const nsRect& aRect)
{
    gfxRect r(GFX_RECT_FROM_TWIPS_RECT(aRect));

    /* Clamp coordinates to work around a design bug in cairo */
    nscoord bigval = (nscoord)(CAIRO_COORD_MAX*mP2A);
    if (aRect.width > bigval ||
        aRect.height > bigval ||
        aRect.x < -bigval ||
        aRect.x > bigval ||
        aRect.y < -bigval ||
        aRect.y > bigval)
    {
        gfxMatrix mat = mThebes->CurrentMatrix();

        r = mat.Transform(r);

        if (!ConditionRect(r))
            return;

        mThebes->SetMatrix(gfxMatrix());
        mThebes->NewPath();

        mThebes->Rectangle(r, true);
        mThebes->Fill();
        mThebes->SetMatrix(mat);
    }

    mThebes->NewPath();
    mThebes->Rectangle(r, true);
    mThebes->Fill();
}

While porting callers of this function to Moz2D I'm making no effort to replicate this. We should probably move this logic into DrawTargetCairo::FillRect though.
Any objections, Bas?
Flags: needinfo?(bas)
Summary: Port the → Port the CAIRO_COORD_MAX code in nsRenderingContext::FillRect to DrawTargetCairo::FillRect
Nope.
Flags: needinfo?(bas)
Seems like we need this after all. :( See bug 1090947.
Comment on attachment 8514917 [details] [diff] [review]
patch

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

::: gfx/2d/DrawTargetCairo.cpp
@@ +122,5 @@
> + */
> +static bool
> +ConditionRect(Rect& r) {
> +  // if either x or y is way out of bounds;
> +  // note that we don't handle negative w/h here

nit: single line if's get { }, there's a bunch of these in this function.

@@ +921,5 @@
> +  Matrix mat;
> +  Rect r = aRect;
> +
> +  /* Clamp coordinates to work around a design bug in cairo */
> +  if (r.width > CAIRO_COORD_MAX ||

This worries me, this is 6 floating point comparisons for every fillrect call.. That's sort of wasteful, but I guess since it's only on cairo it's not too bad.
Attachment #8514917 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/661c58109d16
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.