WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
106267
[chromium] Add OpaqueRegionSkia::currentTrackingOpaqueRect
https://bugs.webkit.org/show_bug.cgi?id=106267
Summary
[chromium] Add OpaqueRegionSkia::currentTrackingOpaqueRect
Alok Priyadarshi
Reported
2013-01-07 15:38:49 PST
Minor implementation cleanup. Make currentTrackingOpaqueRect() a member function of OpaqueRegionSkia so that we do not need to pass member variables everywhere.
Attachments
Patch
(4.66 KB, patch)
2013-01-07 16:03 PST
,
Alok Priyadarshi
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alok Priyadarshi
Comment 1
2013-01-07 16:03:31 PST
Created
attachment 181590
[details]
Patch
Dana Jansens
Comment 2
2013-01-08 10:06:39 PST
Comment on
attachment 181590
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=181590&action=review
> Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:431 > +SkRect& OpaqueRegionSkia::currentTrackingOpaqueRect()
This is fine.. it's adding an extra function call to the stack though, instead of using the inline method. Is this worth it for some reason? I'm not sure of the motivation here. What's wrong with the current method?
Alok Priyadarshi
Comment 3
2013-01-08 10:38:49 PST
This patch comes from
https://bugs.webkit.org/show_bug.cgi?id=105051
, which I decided to abandon. I am trying to extract all the minor cleanup changes and land them separately. The motivation is to reduce duplicated code and potential for bugs. The function arguments (m_opaqueRect, m_canvasLayerStack) are duplicated in all call sites.
Dana Jansens
Comment 4
2013-01-08 10:40:39 PST
(In reply to
comment #3
)
> This patch comes from
https://bugs.webkit.org/show_bug.cgi?id=105051
, which I decided to abandon. I am trying to extract all the minor cleanup changes and land them separately. > > The motivation is to reduce duplicated code and potential for bugs. The function arguments (m_opaqueRect, m_canvasLayerStack) are duplicated in all call sites.
I don't see this as a particularly dangerous pattern, but I don't feel strongly either way. I'll defer to senorblanco.
Stephen White
Comment 5
2013-01-08 11:02:05 PST
Comment on
attachment 181590
[details]
Patch Seems ok. r=me
WebKit Review Bot
Comment 6
2013-01-08 11:17:18 PST
Comment on
attachment 181590
[details]
Patch Clearing flags on attachment: 181590 Committed
r139083
: <
http://trac.webkit.org/changeset/139083
>
WebKit Review Bot
Comment 7
2013-01-08 11:17:21 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug