WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
58063
CTLine objects should outlive their CTRuns
https://bugs.webkit.org/show_bug.cgi?id=58063
Summary
CTLine objects should outlive their CTRuns
Ned Holbrook
Reported
2011-04-07 11:02:18 PDT
CoreText may need to perform some extra work if a CTLine object is released prior to its CTRuns, but this extra work can be avoided by either retaining CTLines longer than their CTRuns (in ComplexTextController), or by not retaining the CTRuns (in ComplexTextRun).
Attachments
Proposed changes.
(2.08 KB, patch)
2011-04-07 16:44 PDT
,
Ned Holbrook
no flags
Details
Formatted Diff
Diff
Remove m_coreTextRun.
(4.38 KB, patch)
2011-04-07 17:50 PDT
,
Ned Holbrook
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Ned Holbrook
Comment 1
2011-04-07 11:04:40 PDT
The latter approach is less desirable since it would eliminate the possibility of using the CTRunGet*Ptr() accessors, so I'll work up some numbers for the former.
Ned Holbrook
Comment 2
2011-04-07 16:44:28 PDT
Created
attachment 88732
[details]
Proposed changes. Numbers are a bit variable on my machine, but I'm seeing an improvement of around 2% for repeated optimizeLegibility test runs on Latin text—barely above the level of noise but certainly not a slowdown.
mitz
Comment 3
2011-04-07 17:07:14 PDT
Comment on
attachment 88732
[details]
Proposed changes. If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr?
Ned Holbrook
Comment 4
2011-04-07 17:09:14 PDT
(In reply to
comment #3
)
> (From update of
attachment 88732
[details]
) > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr?
Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice?
mitz
Comment 5
2011-04-07 17:15:51 PDT
(In reply to
comment #4
)
> (In reply to
comment #3
) > > (From update of
attachment 88732
[details]
[details]) > > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? > > Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice?
Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good.
Ned Holbrook
Comment 6
2011-04-07 17:31:18 PDT
(In reply to
comment #5
)
> Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good.
Theoretically it could be removed entirely since it's only used in the constructor.
mitz
Comment 7
2011-04-07 17:33:39 PDT
(In reply to
comment #6
)
> (In reply to
comment #5
) > > Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. > > Theoretically it could be removed entirely since it's only used in the constructor.
Let’s remove it in practice, then.
Ned Holbrook
Comment 8
2011-04-07 17:50:00 PDT
Created
attachment 88743
[details]
Remove m_coreTextRun.
WebKit Commit Bot
Comment 9
2011-04-07 21:38:56 PDT
Comment on
attachment 88743
[details]
Remove m_coreTextRun. Clearing flags on attachment: 88743 Committed
r83251
: <
http://trac.webkit.org/changeset/83251
>
WebKit Commit Bot
Comment 10
2011-04-07 21:39:00 PDT
All reviewed patches have been landed. Closing bug.
mitz
Comment 11
2011-04-10 09:11:55 PDT
<
rdar://problem/9250245
>
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