New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Jun 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 133548: Skia clipping issue with PDF backend when drawing to the margins

Reported by asvitk...@chromium.org, Jun 19 2012 Project Member

Issue description

I am seeing a problem with text being clipped when drawing via drawPosText() to the PDF margin area:

(i.e. to: device->setDrawingArea(SkPDFDevice::kMargin_DrawingArea))

I believe xji@ and arthurhsu@ investigated this before but ended up working around it instead of finding the root cause.

I believe the issue is that if the clip rect is outside of the PDF content area, it will clip out anything drawn to the margin (since by definition those draw operations go outside of the content area).

Assigning to vandebo@ per reed@'s suggestion.

Note: Currently the code that hits this is not yet landed (it is my RenderTextMac implementation and changes to print_web_view_helper.cc to use it.)

Steve, is there a good place to write a standalone test case for this? Mike mentioned Skia's "SampleApp", but that doesn't seem to do PDF stuff...
 

Comment 1 by reed@google.com, Jun 19 2012

Cc: bunge...@chromium.org
... adding Ben, in case there is a related issue with our XPS backend.

Comment 2 by vandebo@chromium.org, Jun 19 2012

Cc: vandebo@chromium.org
Owner: asvitk...@chromium.org
You should be able to write a gm test to demonstrate the problem. I think gm/shaderbounds.cpp demonstrates all the infrastructure you'll need.  Based on the debugging that arthur did, it's not clear if the problem is in Skia or in RenderText, so if you can make a gm test that reproduces the problem I'll be happy to work on it.

Comment 3 by asvitk...@chromium.org, Jun 19 2012

Is there are a HOWTO somewhere on how to make a gm test?

Comment 4 by vandebo@chromium.org, Jun 19 2012

It's mostly boiler plate - basically you just need to implement the onDraw method.  You can then run gm -w <out dir> --match <gm name> to run just the gm you're interested in and write the result to out dir.

Comment 5 by reed@google.com, Jun 19 2012

gm/tinybitmap.cpp might be a good one to copy/paste to create your own (or just edit it in place and make that available to use to see the problem.

You can also add extra "tests" within a single file, just need a separate function_ptr/registry global. e.g. strokes.cpp has 3 separate GMs in the same file.

Comment 6 by asvitk...@chromium.org, Jun 19 2012

Is there a safe way to get at the SkPDFDevice in the case when the test is running in that configuration?

I need to do:

      device->setDrawingArea(SkPDFDevice::kMargin_DrawingArea);

Comment 7 by vandebo@chromium.org, Jun 19 2012

Hmm, indeed.  There is some missing infrastructure.  As a temporary work around, you can check SkDevice::getDeviceCapabilities() for kVector_Capability, which is true for both PDF and XPS.  XPS is only run on windows.  If it turns out that there is a problem to be fixed, then we can add infrastructure to gm to do the right thing.

Comment 8 by asvitk...@chromium.org, Jun 19 2012

Okay. I guess in my case I need to hack things around to ensure that the SkPDFDevice() is constructed with a pageSize > contentSize too (right now, they're the same).

Comment 9 by vandebo@chromium.org, Jun 19 2012

I think you can use onGetInitialTransform() for that part, that's why I pointed to gm/shaderbounds.cpp

Comment 10 by asvitk...@chromium.org, Jun 20 2012

Cc: -vandebo@chromium.org asvitk...@chromium.org
Owner: vandebo@chromium.org
Here's a repro.

You'll probably want to run it on Mac OS 10.6 to guarantee that it works for you since I use drawPosText() with hardcoded glyph values for the Lucida Grande typeface.

With "bool clip = false;", the text shows up, with "bool clip = false;", it doesn't.

Notice that the clip rect should cover all the glyphs so it shouldn't have any effect on whether the text was drawn or not.

Assigning back to vandebo@ since you now have a repro to work with!
tinybitmap.cpp
2.7 KB View Download

Comment 11 by asvitk...@chromium.org, Jun 20 2012

Sorry, that should have said:

With "bool clip = false;", the text shows up, with "bool clip = true;", it doesn't.

Comment 12 by vandebo@chromium.org, Jun 20 2012

Thanks for the reduced test case.  I understand the issue now.  Unfortunately, there isn't an easy or quick fix.  As long as part of your clip is within the content area, things should work.  Can you use that work around for now?

I suspect setting the clip to a path with two parts, the rectangle that you actually want to clip plus another 1x1 rectangle in the opposite corner of the content area will have the same effect. So you could work around it in a way that shouldn't have any visual differences.

Comment 13 by asvitk...@chromium.org, Jun 22 2012

Cc: msw@chromium.org
The workaround suggested in comment 12 won't work for this case since the clipping is done in a place that doesn't know anything about the fact that the destination is a PDF.

However, we can work around this issue a different way.

+msw for context for the CL that I'm sending your way

Comment 14 by reed@google.com, Jun 22 2012

Should we (or have we already) file a Skia bug, to track this separately from the chrome workaround? Either as an impl or API bug?

Comment 15 by asvitk...@chromium.org, Jun 22 2012

(I've not filed a Skia bug - I'll leave that in your hands.)

Comment 16 by vandebo@chromium.org, Jun 22 2012

I'll file one on Mon or Tue when I get back.

Comment 17 by bugdroid1@chromium.org, Jun 22 2012

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=143726

------------------------------------------------------------------------
r143726 | asvitkine@chromium.org | Fri Jun 22 15:15:43 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/render_text.cc?r1=143726&r2=143725&pathrev=143726
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/render_text.h?r1=143726&r2=143725&pathrev=143726

RenderText: Expose a flag to disable clipping to workaround a Skia bug in the PDF path.

(See bug for context.)

BUG= 133548 
TEST=Verify that the clipping problem is solved with (currently unlanded) RenderTextMac code in print preview.

Review URL: https://chromiumcodereview.appspot.com/10638015
------------------------------------------------------------------------

Comment 18 by bugdroid1@chromium.org, Jun 28 2012

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=144795

------------------------------------------------------------------------
r144795 | asvitkine@chromium.org | Thu Jun 28 14:11:05 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper.cc?r1=144795&r2=144794&pathrev=144795

Workaround Skia clipping bug in print_web_view_helper.cc.

BUG= 133548 
TEST=Test that text isn't clipped incorrectly when enabling RenderTextMac code in print_web_view_helper.cc 

Review URL: https://chromiumcodereview.appspot.com/10703027
------------------------------------------------------------------------

Comment 19 by bugdroid1@chromium.org, Nov 13 2012

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=167311

------------------------------------------------------------------------
r167311 | vitalybuka@chromium.org | 2012-11-13T06:21:54.723607Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper_win.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/browser_resources.grd?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper_linux.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper.h?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/print_web_view_helper_mac.mm?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/printing/print_settings_initializer.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/printing/print_job_constants.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/printing/units.cc?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/printing/print_job_constants.h?r1=167311&r2=167310&pathrev=167311
   M http://src.chromium.org/viewvc/chrome/trunk/src/printing/units.h?r1=167311&r2=167310&pathrev=167311
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/print_preview/print_preview_page.html?r1=167311&r2=167310&pathrev=167311

Print headers and footers with WebKit.

Old implementation with gfx::RenderText had issues with fallback fonts. Sandbox does not allow to read required information from registry.

Also WebKit inplementation is smaller and more readable.

BUG= 152893 ,  108599 ,  133548 
TEST=manual: make sure that main content with or without headers is in the same place (default margin is exception). Make sure that any custom margins and paper layout produce reasonable result. If margins are to small, header and footer should be hidden.

Review URL: https://chromiumcodereview.appspot.com/11359020
------------------------------------------------------------------------

Comment 20 by vitalyb...@chromium.org, Nov 13 2012

r167311 does not fix this issue, it just removed code that had problems because of this this issue.

Comment 21 by bugdroid1@chromium.org, Nov 17 2012

Project Member
Labels: merge-merged-1312
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=168379

------------------------------------------------------------------------
r168379 | vitalybuka@chromium.org | 2012-11-17T04:44:27.527594Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/printing/print_job_constants.cc?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/printing/units.cc?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/printing/print_job_constants.h?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/printing/units.h?r1=168379&r2=168378&pathrev=168379
   A http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/browser/resources/print_preview/print_preview_page.html?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/renderer/print_web_view_helper_win.cc?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/browser/browser_resources.grd?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/renderer/print_web_view_helper_linux.cc?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/renderer/print_web_view_helper.cc?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/renderer/print_web_view_helper.h?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/renderer/print_web_view_helper_mac.mm?r1=168379&r2=168378&pathrev=168379
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/printing/print_settings_initializer.cc?r1=168379&r2=168378&pathrev=168379

Merge 167311 - Print headers and footers with WebKit.

Old implementation with gfx::RenderText had issues with fallback fonts. Sandbox does not allow to read required information from registry.

Also WebKit inplementation is smaller and more readable.

BUG= 152893 ,  108599 ,  133548 
TEST=manual: make sure that main content with or without headers is in the same place (default margin is exception). Make sure that any custom margins and paper layout produce reasonable result. If margins are to small, header and footer should be hidden.

Review URL: https://chromiumcodereview.appspot.com/11359020

TBR=vitalybuka@chromium.org
Review URL: https://codereview.chromium.org/11316066
------------------------------------------------------------------------

Comment 22 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-Internals Cr-Internals

Comment 23 by msw@chromium.org, Aug 15 2013

Is this issue Fixed, and can RenderText's clip_to_display_rect be removed?

Comment 24 by vandebo@chromium.org, Aug 15 2013

I do not think the issue has been fixed.  The one consumer I am aware of, headers and footers in print preview, now works a different way though.  Code search does not show any callers of clip_to_display_rect, so it is probably safe to remove.

Comment 25 by vandebo@chromium.org, Jun 23 2014

Status: WontFix
The preferred way to do margins is now to handle it caller side with a transform, so marking this won't fix.

Sign in to add a comment