Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(89)

Issue 183150: Add smart pointer to Skia

Can't Edit
Can't Publish+Mail
Start Review
Created:
16 years ago by andrei
Modified:
15 years, 9 months ago
Reviewers:
reed, agl, duzy
CC:
andrei
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

The first attached patch adds a smart pointer template to Skia. The second attached patch refactors SkPaint to use this new SkRefPtr template.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Refactor SkPaint to use new SkRefPtr template #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -34 lines) Patch
M include/core/SkPaint.h View 9 chunks +16 lines, -16 lines 0 comments Download
M src/core/SkPaint.cpp View 3 chunks +9 lines, -18 lines 0 comments Download

Messages

Total messages: 4
andrei
I propose adding a smart pointer template to Skia and would appreciate your feedback. Thank ...
16 years ago (2010-01-07 23:42:56 UTC) #1
andrei
Even if the SkRefPtr template is not used internally within Skia, I believe it would ...
15 years, 12 months ago (2010-01-15 00:43:00 UTC) #2
agl
deferring to reed on this one.
15 years, 12 months ago (2010-01-15 09:46:35 UTC) #3
duzy
15 years, 9 months ago (2010-04-16 08:31:03 UTC) #4
I would give a score to smart pointer like this SkRefPtr.

But the SkRefPtr should need some improvement, I think.

http://codereview.appspot.com/183150/diff/1/2
File include/core/SkRefCnt.h (right):

http://codereview.appspot.com/183150/diff/1/2#newcode174
include/core/SkRefCnt.h:174: T* operator->() const { return fObj; }
operator*() and operator->() would expose 'ref()' and 'unref()' from SkRefCnt to
users, this is bad I think, just take look at this segment:

    SkRefPtr<SkTypeface> typeface;
    ...
    ...
    typeface->unref(); // possible, and really bad
    ...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b