anti-aliased clips don't work correctly in the Skia PDF backend |
||||||
Issue descriptionNeed to attach a reduced testcase. crbug.com/590971 comment 0 has repro steps, except for needing to revert https://codereview.chromium.org/1751993004
,
Mar 5 2016
Hal could you fix? Meantime, it seems we don't have the infrastructure to test PDF printing in layout tests. It would be nice to do that, has it been discussed before?
,
Mar 5 2016
Repro steps: 1. Edit ClipDisplayItem.cpp to change line 19 to pass AntiAliased in the second parameter. 2. Build out/Release/chrome 3. Load the above example (comment #1) 4. ctrl-p to print Print preview will show a black rectangle, rather than a white one with a 1px black shadow.
,
Mar 30 2016
I have reduced this to the following Skia GM testcase:
#include "gm.h"
DEF_SIMPLE_GM(crbug_591805, canvas, 173, 173) {
auto bigRect = SkRect::MakeWH(172.8f, 172.8f);
auto rect = SkRect::MakeLTRB(6.4f, 6.4f, 166.4f, 166.4f);
canvas->clipRect(bigRect, SkRegion::kIntersect_Op, true);
canvas->clipRect(rect, SkRegion::kDifference_Op, false);
canvas->translate(0.8f, 0.8f);
canvas->drawRect(rect, SkPaint());
}
SK_PDF_USE_PATHOPS_CLIPPING seems to fix this. I will see if I can turn that on without breaking anything.
,
Mar 30 2016
,
Mar 31 2016
The following revision refers to this bug: https://skia.googlesource.com/skia.git/+/dda239e25b54f4265734c62e2fdc76c872406f28 commit dda239e25b54f4265734c62e2fdc76c872406f28 Author: halcanary <halcanary@google.com> Date: Thu Mar 31 14:33:57 2016 SkPDF: Use Pathops clipping Turn this on all the time. Remove the SK_PDF_USE_PATHOPS_CLIPPING define that used to hide this functionality. All rendering tests are the same or improved by this. Also, remove non-functional SK_ALLOW_LARGE_PDF_SCALARS. TBR=reed@google.com removing dead #defines from SkUserConfig.h BUG= 591805 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1845623002 Review URL: https://codereview.chromium.org/1845623002 [modify] https://crrev.com/dda239e25b54f4265734c62e2fdc76c872406f28/include/config/SkUserConfig.h [modify] https://crrev.com/dda239e25b54f4265734c62e2fdc76c872406f28/src/pdf/SkPDFDevice.cpp
,
Mar 31 2016
Waiting to verify in tomorrow's canary.
,
Apr 4 2016
I just realized that the canary means nothing, since I have to recompile with AntiAliased clips turned on. I just did that with top-of-tree (commit 15ee578), and yes, it works as expected.
,
Apr 4 2016
Also, chrishtr@, with respect to testing the printing workflow, I have an OKR to work on that.
,
Apr 4 2016
Awesome, thanks for fixing. I will now revert https://codereview.chromium.org/1751993004. For the OKR: what form do you plan for the tests to take?
,
Apr 4 2016
I just tried at ToT and the steps reported in comment 0 of issue 590971 still repro.
,
Apr 4 2016
Step one is here: https://crbug.com/599185 . This would let me use existing Skia infrastructure to find regressions in Skia as they happen.
,
Apr 11 2016
I can not reproduce. Can you get me a simple test case?
,
Apr 25 2016
Hmm neither can I any more. Going to revert my patch in Blink now then.
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92ae9ad89fa5eb1e10cd008969418de6867b09b7 commit 92ae9ad89fa5eb1e10cd008969418de6867b09b7 Author: chrishtr <chrishtr@chromium.org> Date: Wed Apr 27 02:04:53 2016 Revert code to avoid AntiAliased clips now that the referenced Skia bug is fixed. BUG= 591805 , 590971 Review URL: https://codereview.chromium.org/1921493006 Cr-Commit-Position: refs/heads/master@{#389971} [modify] https://crrev.com/92ae9ad89fa5eb1e10cd008969418de6867b09b7/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/92ae9ad89fa5eb1e10cd008969418de6867b09b7/third_party/WebKit/Source/platform/graphics/paint/ClipDisplayItem.cpp
,
Apr 28 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by chrishtr@chromium.org
, Mar 5 2016