SkCanvas::addRectURL -- to record URL annotation (destined for printers)

SkCanvas::addRectURL -- to record URL annotation (destined for printers)

[reed1, 2012-06-28 20:50:45]

I think we can add this to device subclasses later

[bungeman, 2012-06-28 20:55:03]

On 2012/06/28 20:51:15, reed1 wrote:

LGTM

[Steve VanDeBogart, 2012-06-28 20:56:12]

LGTM

https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
Do we need to clip the rect to the device's clip here?

[reed1, 2012-06-28 21:18:07]

On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> LGTM
> 
> https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> File src/core/SkCanvas.cpp (right):
> 
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> Do we need to clip the rect to the device's clip here?

Probably right. I can add quickReject here, to entirely skip the URL if its
clipped out. I don't think I should reduce the rect if its partially clipped
though.

[reed1, 2012-06-28 21:22:00]

Alternate crazy idea:

Add a new hook to SkPaint (ala shader) for annotations. This has the advantage
of being a (potentially) smaller change, since I don't have to edit all canvas
subclasses. It has the disadvantage of adding a new hook to SkPaint (which it
already has in abundance).

e.g.

SkPaint::setAnnotation(SkData);
or
SkPaint::setAnnotation(GUID, SkData);
or
SkPaint::setAnnotation(SkMetaData);
or
...?

I'm a little reluctant to create yet-another-virtual-baseclass for this, though
that would be fun. If we can get away with something more declarative, it makes
for an easier time to copy/flatten/etc. the payload.

thoughts?

[Steve VanDeBogart, 2012-06-28 21:25:47]

On 2012/06/28 21:18:07, reed1 wrote:
> On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > LGTM
> > 
> > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > File src/core/SkCanvas.cpp (right):
> > 
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > Do we need to clip the rect to the device's clip here?
> 
> Probably right. I can add quickReject here, to entirely skip the URL if its
> clipped out. I don't think I should reduce the rect if its partially clipped
> though.

Consider a link that's in an iframe scrolled so that only part of the link is
visible... we don't want to make the invisible part of the link clickable...

[Steve VanDeBogart, 2012-06-28 21:26:57]

On 2012/06/28 21:25:47, Steve VanDeBogart wrote:
> On 2012/06/28 21:18:07, reed1 wrote:
> > On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > > LGTM
> > > 
> > > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > > File src/core/SkCanvas.cpp (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > > Do we need to clip the rect to the device's clip here?
> > 
> > Probably right. I can add quickReject here, to entirely skip the URL if its
> > clipped out. I don't think I should reduce the rect if its partially clipped
> > though.
> 
> Consider a link that's in an iframe scrolled so that only part of the link is
> visible... we don't want to make the invisible part of the link clickable...

I think this approach will be harder to fit in with the current WebKit code. 
While the churn of adding stuff to SkCanvas is annoying, I think it's the right
approach in this case.

[reed1, 2012-06-28 21:27:02]

On 2012/06/28 21:25:47, Steve VanDeBogart wrote:
> On 2012/06/28 21:18:07, reed1 wrote:
> > On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > > LGTM
> > > 
> > > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > > File src/core/SkCanvas.cpp (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > > Do we need to clip the rect to the device's clip here?
> > 
> > Probably right. I can add quickReject here, to entirely skip the URL if its
> > clipped out. I don't think I should reduce the rect if its partially clipped
> > though.
> 
> Consider a link that's in an iframe scrolled so that only part of the link is
> visible... we don't want to make the invisible part of the link clickable...

Isn't that the job of the device that "plays back" the annotation? If the rect
is complexly clipped (even antialiased complexly clipped), what data should I
pass to the device?

[reed1, 2012-06-28 21:29:48]

On 2012/06/28 21:26:57, Steve VanDeBogart wrote:
> On 2012/06/28 21:25:47, Steve VanDeBogart wrote:
> > On 2012/06/28 21:18:07, reed1 wrote:
> > > On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > > > LGTM
> > > > 
> > > > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > > > File src/core/SkCanvas.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > > > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > > > Do we need to clip the rect to the device's clip here?
> > > 
> > > Probably right. I can add quickReject here, to entirely skip the URL if
its
> > > clipped out. I don't think I should reduce the rect if its partially
clipped
> > > though.
> > 
> > Consider a link that's in an iframe scrolled so that only part of the link
is
> > visible... we don't want to make the invisible part of the link clickable...
> 
> I think this approach will be harder to fit in with the current WebKit code. 
> While the churn of adding stuff to SkCanvas is annoying, I think it's the
right
> approach in this case.

In the crazy approach, addRectURL would look like this:

addRectURL(rect, url) {
    SkPaint paint;
    paint.setAnnotation(..., url);
    canvas.drawRect(rect, paint);
}

with some mechanism to tell us to *not* actually render the rect (perhaps some
flag on the annotation itself)?

[Steve VanDeBogart, 2012-06-28 21:39:53]

On 2012/06/28 21:27:02, reed1 wrote:
> On 2012/06/28 21:25:47, Steve VanDeBogart wrote:
> > On 2012/06/28 21:18:07, reed1 wrote:
> > > On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > > > LGTM
> > > > 
> > > > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > > > File src/core/SkCanvas.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > > > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > > > Do we need to clip the rect to the device's clip here?
> > > 
> > > Probably right. I can add quickReject here, to entirely skip the URL if
its
> > > clipped out. I don't think I should reduce the rect if its partially
clipped
> > > though.
> > 
> > Consider a link that's in an iframe scrolled so that only part of the link
is
> > visible... we don't want to make the invisible part of the link clickable...
> 
> Isn't that the job of the device that "plays back" the annotation? If the rect
> is complexly clipped (even antialiased complexly clipped), what data should I
> pass to the device?

In PDF, annotations are specified at the page level, i.e. not inline with
drawing, so I don't think clipping applies to them.  It's fair that you can't
apply a complex clip to them.  Looking again, maybe WebKit will handle the case
that I mentioned with the iframe.  So... nevermind this.

[Steve VanDeBogart, 2012-06-28 21:40:26]

On 2012/06/28 21:29:48, reed1 wrote:
> On 2012/06/28 21:26:57, Steve VanDeBogart wrote:
> > On 2012/06/28 21:25:47, Steve VanDeBogart wrote:
> > > On 2012/06/28 21:18:07, reed1 wrote:
> > > > On 2012/06/28 20:56:12, Steve VanDeBogart wrote:
> > > > > LGTM
> > > > > 
> > > > > https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp
> > > > > File src/core/SkCanvas.cpp (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.appspot.com/6354052/diff/1/src/core/SkCanvas.cpp#newcode1906
> > > > > src/core/SkCanvas.cpp:1906: iter.fDevice->addRectURL(rect, url);
> > > > > Do we need to clip the rect to the device's clip here?
> > > > 
> > > > Probably right. I can add quickReject here, to entirely skip the URL if
> its
> > > > clipped out. I don't think I should reduce the rect if its partially
> clipped
> > > > though.
> > > 
> > > Consider a link that's in an iframe scrolled so that only part of the link
> is
> > > visible... we don't want to make the invisible part of the link
clickable...
> > 
> > I think this approach will be harder to fit in with the current WebKit code.

> > While the churn of adding stuff to SkCanvas is annoying, I think it's the
> right
> > approach in this case.
> 
> In the crazy approach, addRectURL would look like this:
> 
> addRectURL(rect, url) {
>     SkPaint paint;
>     paint.setAnnotation(..., url);
>     canvas.drawRect(rect, paint);
> }
> 
> with some mechanism to tell us to *not* actually render the rect (perhaps some
> flag on the annotation itself)?

Hmm, I guess that could work.