New issue
Advanced search Search tips

Issue 1536 link

Starred by 7 users

Issue metadata

Status: Accepted
Owner:
Cc:
Area: PDF , AndroidFramework
NextAction: ----
Priority: Medium
Type: Defect



Sign in to add a comment

SkCanvas::saveLayer flags are not respected by PDF backend

Project Member Reported by djsollen@google.com, Aug 20 2013

Issue description

What steps will reproduce the problem?
1. rungm --match canvas-layer-state -w ./canvas-state
2. view output of ./canvas-state and you'll find that the pdf output does not match the correct output (i.e. 8888)

What is the expected output? What do you see instead?

When saveLayer is called without kClipToLayer_SaveFlag then when we draw outside the layer's bounds the draw calls should propagate to the other layers on the canvas' stack.
 
Project Member

Comment 1 by hcm@google.com, Jan 8 2014

Owner: bungeman@google.com
Project Member

Comment 2 by djsollen@google.com, Mar 20 2014

Labels: OpSys-Android
Project Member

Comment 3 by djsollen@google.com, Jan 28 2015

Cc: djsollen@google.com
Labels: -OpSys-Android
Owner: halcanary@google.com
This is still an issue.  We can either fix it or at a minimum we need to document somewhere the places that the PDF backend differs from the expected behavior.
Project Member

Comment 4 by djsollen@google.com, Jan 28 2015

 Issue 1502  has been merged into this issue.
Project Member

Comment 5 by djsollen@google.com, Jan 28 2015

Labels: -Area-AndroidNext Area-AndroidFramework
Project Member

Comment 6 by halcanary@google.com, Aug 22 2016

Status: Unconfirmed (was: Accepted)
Here's the old canvas-layer-state gm, translated into modern parlance (I think).  PDF and 8888 render the same.

    #include "gm.h"
    const int kWidth = 400;
    const int kHeight = 400;
    DEF_SIMPLE_GM(canvas_layer_state, canvas, kWidth, kHeight) {
        const int kSpacer = 10.0f;
        SkPaint bluePaint;
        SkRect rect;
        bluePaint.setColor(SK_ColorBLUE);
        bluePaint.setStyle(SkPaint::kFill_Style);

        rect = SkRect::MakeXYWH(
                kSpacer, kSpacer, kWidth - (2 * kSpacer),
                (kHeight - 7 * kSpacer) / 6.0f);
        SkScalar yShift = rect.height() + kSpacer;
        canvas->drawColor(SK_ColorRED);
        struct { uint8_t layerAlpha; bool cropLayer; } tests[] = {
            {255, false},
            {255, true},
            {0, false},
        };
        for (auto test : tests) {
            SkAutoCanvasRestore autoCanvasRestore(canvas, false);
            if (test.cropLayer) {
                canvas->saveLayerAlpha(&rect, test.layerAlpha);
                canvas->clipRect(rect);
            } else {
                canvas->saveLayerAlpha(nullptr, test.layerAlpha);
            }
            canvas->drawRect(rect, bluePaint);
            rect.offset(0, yShift);
            canvas->drawRect(rect, bluePaint);
            rect.offset(0, yShift);
        }
    }

Is this fixed now?
Project Member

Comment 7 by djsollen@google.com, Aug 22 2016

I don't think you can replicate this behavior with the existing API unless you build with the SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG define.  The intent of this flag is that if you do the following...

SaveLayerRec rec(SkRect(50,50,100,100), nullptr, kDontClipToLayer_Legacy_SaveLayerFlag);
canvas->saveLayer(rec);
canvas->drawRect(-50,-50,100,100, RED_SKPAINT);
canvas->restore();

The result on the original canvas would be a 100x100 rect starting at (0,0) instead of a 50x50 rect at (50,50) if drawn without the flag.  The output gets even more bizarre if the saveLayer had a paint with a xfermode applied to it.

This flag isn't a behavior we want to encourage so I'm fine if we don't put resources into fixing it now.  On the other hand I don't think we can close it out because if we can't find an acceptable alternative for Android we will have to fix it.
Project Member

Comment 8 by halcanary@google.com, Aug 22 2016

Status: Accepted (was: Unconfirmed)
I see.  https://fiddle.skia.org/c/3045851bcb2d9a430f1ce4cd9ab31bdb

    #include "gm.h"
    const int kWidth = 400;
    const int kHeight = 400;
    DEF_SIMPLE_GM(canvas_layer_state, canvas, kWidth, kHeight) {
        const int kSpacer = 10.0f;
        SkPaint bluePaint;
        bluePaint.setColor(SK_ColorBLUE);
        bluePaint.setStyle(SkPaint::kFill_Style);
        SkRect rect = SkRect::MakeXYWH(
                kSpacer, kSpacer, kWidth - (2 * kSpacer),
                (kHeight - 7 * kSpacer) / 6.0f);
        SkScalar yShift = rect.height() + kSpacer;
        canvas->drawColor(SK_ColorRED);
        struct { uint8_t layerAlpha; bool cropLayer; } tests[] = {
            {192, false},
            {192, true},
            {64, false},
        };
        for (auto test : tests) {
            SkAutoCanvasRestore autoCanvasRestore(canvas, false);
            SkPaint alphaPaint;
            alphaPaint.setAlpha(test.layerAlpha);
            if (test.cropLayer) {
                canvas->saveLayer(SkCanvas::SaveLayerRec(&rect, &alphaPaint, 0));
                canvas->clipRect(rect);
            } else {
                #ifdef SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG
                uint32_t flag = SkCanvas::kDontClipToLayer_Legacy_SaveLayerFlag;
                SkRect* clipRect = ▭
                #else
                uint32_t flag = 0;
                SkRect* clipRect = nullptr;
                #endif
                canvas->saveLayer(SkCanvas::SaveLayerRec(clipRect, &alphaPaint, flag));
            }
            canvas->drawRect(rect, bluePaint);
            rect.offset(0, yShift);
            canvas->drawRect(rect, bluePaint);
            rect.offset(0, yShift);
        }
    }

Sign in to add a comment