SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

SkPDFFont::countStretch() removed, and the stretch loop flattened.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002

Committed: https://skia.googlesource.com/skia/+/c2f9ec1f5e4e8e98489cd5ea9356771cf4d8ce13

[hal.canary, 2016-09-09 22:35:05]

Description was changed from

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

BUG=skia:5434
==========

to

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

[hal.canary, 2016-09-09 22:35:11]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 22:35:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hal.canary, 2016-09-09 22:36:20]

Description was changed from

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

to

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

[commit-bot: I haz the power, 2016-09-09 22:38:04]

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

[commit-bot: I haz the power, 2016-09-09 22:38:05]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[hal.canary, 2016-09-09 22:39:54]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 22:40:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hal.canary, 2016-09-09 22:51:52]

halcanary@google.com changed reviewers:
+ tomhudson@google.com

[hal.canary, 2016-09-09 22:51:52]

PTAL

[commit-bot: I haz the power, 2016-09-09 23:13:54]

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

[commit-bot: I haz the power, 2016-09-09 23:13:55]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-09-12 11:56:15]

Patchset #1 (id:1) has been deleted

[hal.canary, 2016-09-12 12:14:28]

Description was changed from

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

to

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

SkPDFFont::countStretch() removed, and the stretch loop flattened.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

[tomhudson, 2016-09-12 14:49:10]

very LGTM

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:862: , fCurrentMatrixOrigin(origin)
Nice bugfix! (?)

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:877: if (!fInitialized) {
Are we deferring this because we sometimes create empty GlyphPositioner objects?

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1096: // Try to get a font which can encode the next
glyph.
nit: "this" glyph for consistency with previous comment?

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1109: if (!defaultPositioning) {
nice simplification

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFResourceDi...
File src/pdf/SkPDFResourceDict.cpp (right):

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFResourceDi...
src/pdf/SkPDFResourceDict.cpp:53: return SkStringPrintf("%c%d",
SkPDFResourceDict::GetResourceTypePrefix(type), key);
Why was making the function part of the API necessary for this temp-elimination?

[hal.canary, 2016-09-12 14:59:02]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-09-12 14:59:03]

The patchset sent to the CQ was uploaded after l-g-t-m from tomhudson@google.com
Link to the patchset: https://codereview.chromium.org/2327953002/#ps60001
(title: "comments")

[commit-bot: I haz the power, 2016-09-12 14:59:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hal.canary, 2016-09-12 14:59:24]

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:862: , fCurrentMatrixOrigin(origin)
On 2016/09/12 14:49:09, tomhudson wrote:
> Nice bugfix! (?)

Not a bugfix.  The output is identical.

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:877: if (!fInitialized) {
On 2016/09/12 14:49:09, tomhudson wrote:
> Are we deferring this because we sometimes create empty GlyphPositioner
objects?

Because I want the font info serialized first.  But that too (if all glyphs are
bad).

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1096: // Try to get a font which can encode the next
glyph.
On 2016/09/12 14:49:09, tomhudson wrote:
> nit: "this" glyph for consistency with previous comment?

Done.

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1109: if (!defaultPositioning) {
On 2016/09/12 14:49:09, tomhudson wrote:
> nice simplification

Acknowledged.

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFResourceDi...
File src/pdf/SkPDFResourceDict.cpp (right):

https://codereview.chromium.org/2327953002/diff/40001/src/pdf/SkPDFResourceDi...
src/pdf/SkPDFResourceDict.cpp:53: return SkStringPrintf("%c%d",
SkPDFResourceDict::GetResourceTypePrefix(type), key);
On 2016/09/12 14:49:09, tomhudson wrote:
> Why was making the function part of the API necessary for this
temp-elimination?

This function is functionally unchanged, and I still use it somewhere else.  It
was written before SkStringPrintf() existed, so I cleaned that up.

[commit-bot: I haz the power, 2016-09-12 15:55:32]

Description was changed from

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

SkPDFFont::countStretch() removed, and the stretch loop flattened.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002
==========

to

==========
SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002

SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and
fTextSize.

SkPDFDevice::updateFont(): replace with update_font() and inlined
code.  De-duplicate this block of code.

SkPDFResourceDict::GetResourceTypePrefix function made public: removes
need for temporary SkString returned by
SkPDFResourceDict::getResourceName()

GlyphPositioner: delay writing intial matrix until first glyph.
Assert that widechars is a constant.

SkPDFFont::FontType(): make public so that PDFDevice can know about
multibyte status.

SkPDFFont::countStretch() removed, and the stretch loop flattened.

*no changes in PDF output*

BUG=skia:5434
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002

Committed:
https://skia.googlesource.com/skia/+/c2f9ec1f5e4e8e98489cd5ea9356771cf4d8ce13
==========

[commit-bot: I haz the power, 2016-09-12 15:55:34]

Committed patchset #3 (id:60001) as
https://skia.googlesource.com/skia/+/c2f9ec1f5e4e8e98489cd5ea9356771cf4d8ce13