Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(265)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 6 months ago by reed1
Modified:
13 years, 6 months ago
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.
SkCanvas::addRectURL -- to record URL annotation (destined for printers)

Patch Set 1 #

Total comments: 1
Created: 13 years, 6 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -18 lines) Patch
M include/core/SkCanvas.h View 2 chunks +9 lines, -0 lines 0 comments Download
M include/core/SkDevice.h View 1 chunk +2 lines, -0 lines 0 comments Download
M include/core/SkStream.h View 1 chunk +1 line, -1 line 0 comments Download
M include/utils/SkDumpCanvas.h View 2 chunks +3 lines, -1 line 0 comments Download
M include/utils/SkNWayCanvas.h View 1 chunk +2 lines, -0 lines 0 comments Download
M include/utils/SkProxyCanvas.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/core/SkCanvas.cpp View 1 chunk +11 lines, -0 lines 1 comment Download
M src/core/SkDevice.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M src/core/SkPictureFlat.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/core/SkPicturePlayback.h View 4 chunks +9 lines, -1 line 0 comments Download
M src/core/SkPicturePlayback.cpp View 9 chunks +60 lines, -7 lines 0 comments Download
M src/core/SkPictureRecord.h View 4 chunks +6 lines, -0 lines 0 comments Download
M src/core/SkPictureRecord.cpp View 4 chunks +21 lines, -0 lines 0 comments Download
M src/utils/SkDumpCanvas.cpp View 2 chunks +8 lines, -0 lines 0 comments Download
M src/utils/SkNWayCanvas.cpp View 1 chunk +14 lines, -0 lines 0 comments Download
M src/utils/SkProxyCanvas.cpp View 1 chunk +4 lines, -0 lines 0 comments Download
M tests/CanvasTest.cpp View 3 chunks +78 lines, -8 lines 0 comments Download
Total messages: 13
|
reed1
13 years, 6 months ago (2012年06月28日 20:50:24 UTC) #1
Sign in to reply to this message.
reed1
I think we can add this to device subclasses later
13 years, 6 months ago (2012年06月28日 20:50:45 UTC) #2
I think we can add this to device subclasses later
Sign in to reply to this message.
reed1
13 years, 6 months ago (2012年06月28日 20:51:15 UTC) #3
Sign in to reply to this message.
bungeman
On 2012年06月28日 20:51:15, reed1 wrote: LGTM
13 years, 6 months ago (2012年06月28日 20:55:03 UTC) #4
On 2012年06月28日 20:51:15, reed1 wrote:
LGTM
Sign in to reply to this message.
Steve VanDeBogart
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 ...
13 years, 6 months ago (2012年06月28日 20:56:12 UTC) #5
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?
Sign in to reply to this message.
reed1
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): ...
13 years, 6 months ago (2012年06月28日 21:18:07 UTC) #6
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.
Sign in to reply to this message.
reed1
Alternate crazy idea: Add a new hook to SkPaint (ala shader) for annotations. This has ...
13 years, 6 months ago (2012年06月28日 21:22:00 UTC) #7
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?
Sign in to reply to this message.
Steve VanDeBogart
On 2012年06月28日 21:18:07, reed1 wrote: > On 2012年06月28日 20:56:12, Steve VanDeBogart wrote: > > LGTM ...
13 years, 6 months ago (2012年06月28日 21:25:47 UTC) #8
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...
Sign in to reply to this message.
Steve VanDeBogart
On 2012年06月28日 21:25:47, Steve VanDeBogart wrote: > On 2012年06月28日 21:18:07, reed1 wrote: > > On ...
13 years, 6 months ago (2012年06月28日 21:26:57 UTC) #9
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.
Sign in to reply to this message.
reed1
On 2012年06月28日 21:25:47, Steve VanDeBogart wrote: > On 2012年06月28日 21:18:07, reed1 wrote: > > On ...
13 years, 6 months ago (2012年06月28日 21:27:02 UTC) #10
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?
Sign in to reply to this message.
reed1
On 2012年06月28日 21:26:57, Steve VanDeBogart wrote: > On 2012年06月28日 21:25:47, Steve VanDeBogart wrote: > > ...
13 years, 6 months ago (2012年06月28日 21:29:48 UTC) #11
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)?
Sign in to reply to this message.
Steve VanDeBogart
On 2012年06月28日 21:27:02, reed1 wrote: > On 2012年06月28日 21:25:47, Steve VanDeBogart wrote: > > On ...
13 years, 6 months ago (2012年06月28日 21:39:53 UTC) #12
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.
Sign in to reply to this message.
Steve VanDeBogart
On 2012年06月28日 21:29:48, reed1 wrote: > On 2012年06月28日 21:26:57, Steve VanDeBogart wrote: > > On ...
13 years, 6 months ago (2012年06月28日 21:40:26 UTC) #13
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.
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /