RenderTextMac: Cache the SkTypeface on the TextRun

RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136
Committed: https://crrev.com/758d2baa307aa32b9de708e97655663726b8fa9c
Cr-Commit-Position: refs/heads/master@{#389042}

[tapted, 2016-04-21 04:08:23]

Patchset #2 (id:20001) has been deleted

[tapted, 2016-04-21 06:42:37]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

BUG=605131
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. It's only used on Mac. Also, there was actually never anything to
derive on Mac, since RenderTextMac does (on every *paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131
==========

[tapted, 2016-04-21 06:42:37]

tapted@chromium.org changed reviewers:
+ asvitkine@chromium.org

[tapted, 2016-04-21 06:55:43]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. It's only used on Mac. Also, there was actually never anything to
derive on Mac, since RenderTextMac does (on every *paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131
==========

[tapted, 2016-04-21 11:55:18]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131, 605136
==========

[tapted, 2016-04-21 11:55:51]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # This is a "no-op", but
expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131, 605136
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131, 605136
==========

[tapted, 2016-04-21 12:06:41]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun the way
RenderTextHarfbuzz does.

BUG=605131, 605136
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136
==========

[tapted, 2016-04-21 12:07:14]

Hi Alexei, please take a look

[Alexei Svitkine (slow), 2016-04-21 14:48:30]

lgtm

[tapted, 2016-04-21 23:25:03]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-04-21 23:25:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907833002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907833002/40001

[commit-bot: I haz the power, 2016-04-21 23:35:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-21 23:35:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tapted, 2016-04-22 06:13:57]

ugh, shortly after uploading this r388275 added a new presubmit that banned
new/changed/moved code from using skia::Refptr, in favour of sk_sp. The
conversion is pretty simple though -- it just does away with 
"AdoptRef" in favour of what we're used to from other shared_ptr types, which
"adopt" in the constructor or .reset

[tapted, 2016-04-22 06:14:04]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-04-22 06:14:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1907833002/#ps80001
(title: "skia::RefPtr<>(Adopt(..)) -> sk_sp<>(..)")

[commit-bot: I haz the power, 2016-04-22 06:14:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907833002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907833002/80001

[commit-bot: I haz the power, 2016-04-22 06:19:17]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136
==========

[commit-bot: I haz the power, 2016-04-22 06:19:18]

Committed patchset #4 (id:80001)

[kjellander_chromium, 2016-04-22 11:23:31]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/1914443002/ by kjellander@chromium.org.

The reason for reverting is: Seems to break gfx_unittests on Mac10.11 Tests:
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/1384

See
https://findit-for-me.appspot.com/build-failure?url=https://build.chromium.or...
for more info..

[commit-bot: I haz the power, 2016-04-22 19:44:53]

Description was changed from

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136
==========

to

==========
RenderTextMac: Cache the SkTypeface on the TextRun

SkiaTextRenderer::SetFontWithStyle() uses gfx::Font::Derive() which is
slow. SetFontWithStyle is only used on Mac but, also, there was actually
never anything to derive on Mac, since RenderTextMac does (on every
*paint*):
  CTFontRef = <get font from CTRun>
  gfx::Font = <wrap CTFontRef>
  CFFontRef = <unwrap from gfx:Font>  # cheap
  style = <extract style from CTFontRef>
  gfx::Font_2 = Derive(apply <style> to gfx::Font)  # A "no-op", but expensive

So remove SkiaTextRenderer::SetFontWithStyle() since nothing needs it.

Then store the SkTypeface on the RenderTextMac TextRun. This saves a
lookup on each paint and is consistent with RenderTextHarfbuzz.

BUG=605131, 605136

Committed: https://crrev.com/758d2baa307aa32b9de708e97655663726b8fa9c
Cr-Commit-Position: refs/heads/master@{#389042}
==========

[commit-bot: I haz the power, 2016-04-22 19:44:54]

Patchset 4 (id:??) landed as
https://crrev.com/758d2baa307aa32b9de708e97655663726b8fa9c
Cr-Commit-Position: refs/heads/master@{#389042}

[tapted, 2016-04-26 23:59:27]

In case anyone comes spelunking here, this was continued in
https://codereview.chromium.org/1919123002 and landed in r389924 as
https://crrev.com/59fe54df8c0de55f03c8fb5e1860279d2993b473

Details about the regression: http://crrev.com/1919123002#msg3