Conservative SkTextBlob bounds.

Conservative SkTextBlob bounds.

Compute cheaper/more conservative text blob bounds based on the typeface
maximum glyph bbox.

BUG=chromium:451401
R=reed@google.com,bungeman@google.com

Committed: https://skia.googlesource.com/skia/+/3dc40ac9f968eee95eef5e8ee811e0640691df0f

[f(malita), 2015-01-28 14:46:19]

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp
File src/core/SkTextBlob.cpp (right):

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp#newc...
src/core/SkTextBlob.cpp:382: }
Here used to live an awesome assert: conservative bounds should always contain
the tight bounds.

Except this awesome assert was firing in Chromium for certain pages, because of
minor vertical tight box snapping due to hinting (as Ben explained).

So, err, the awesome assert no longer lives here...

[mtklein, 2015-01-28 14:58:21]

mtklein@google.com changed reviewers:
+ mtklein@google.com

[mtklein, 2015-01-28 14:58:21]

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp
File src/core/SkTextBlob.cpp (right):

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp#newc...
src/core/SkTextBlob.cpp:382: }
On 2015/01/28 14:46:19, f(malita) wrote:
> Here used to live an awesome assert: conservative bounds should always contain
> the tight bounds.
> 
> Except this awesome assert was firing in Chromium for certain pages, because
of
> minor vertical tight box snapping due to hinting (as Ben explained).
> 
> So, err, the awesome assert no longer lives here...

I actually had the same sort of problem over in the bbh building world (no
surprise), and fixing it was the reason we added getBounds() to SkTypeface. 
Turns out, SkTypeface::getBounds() isn't quite good enough in all cases.  It
doesn't account for the paint effects on that box, like x scale and skew.  This
led to a few rendering issues where small skewed text was dropped.

Luckily, we now have SkPaint::getFontBounds(), which applies the scale and skew
and should return the rectangle you actually want.  gm/fontmgr demos it.  I'm
not really sure why I haven't made use of it yet over in the BBH code...

[reed1, 2015-01-28 15:02:22]

https://codereview.chromium.org/886473002/diff/1/include/core/SkTextBlob.h
File include/core/SkTextBlob.h (right):

https://codereview.chromium.org/886473002/diff/1/include/core/SkTextBlob.h#ne...
include/core/SkTextBlob.h:191: static SkRect conservativeRunBounds(const
SkTextBlob::RunRecord&);
nit: static functions are Capitalized()

[f(malita), 2015-01-28 15:14:37]

https://codereview.chromium.org/886473002/diff/1/include/core/SkTextBlob.h
File include/core/SkTextBlob.h (right):

https://codereview.chromium.org/886473002/diff/1/include/core/SkTextBlob.h#ne...
include/core/SkTextBlob.h:191: static SkRect conservativeRunBounds(const
SkTextBlob::RunRecord&);
On 2015/01/28 15:02:22, reed1 wrote:
> nit: static functions are Capitalized()

Done.

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp
File src/core/SkTextBlob.cpp (right):

https://codereview.chromium.org/886473002/diff/1/src/core/SkTextBlob.cpp#newc...
src/core/SkTextBlob.cpp:382: }
On 2015/01/28 14:58:21, mtklein wrote:
> On 2015/01/28 14:46:19, f(malita) wrote:
> > Here used to live an awesome assert: conservative bounds should always
contain
> > the tight bounds.
> > 
> > Except this awesome assert was firing in Chromium for certain pages, because
> of
> > minor vertical tight box snapping due to hinting (as Ben explained).
> > 
> > So, err, the awesome assert no longer lives here...
> 
> I actually had the same sort of problem over in the bbh building world (no
> surprise), and fixing it was the reason we added getBounds() to SkTypeface. 
> Turns out, SkTypeface::getBounds() isn't quite good enough in all cases.  It
> doesn't account for the paint effects on that box, like x scale and skew. 
This
> led to a few rendering issues where small skewed text was dropped.
> 
> Luckily, we now have SkPaint::getFontBounds(), which applies the scale and
skew
> and should return the rectangle you actually want.  gm/fontmgr demos it.  I'm
> not really sure why I haven't made use of it yet over in the BBH code...

Yay for getFontBounds() - thanks for the tip! Done.

[reed1, 2015-01-28 15:15:32]

lgtm

[mtklein, 2015-01-28 15:18:54]

lgtm

I do remember now why I didn't use it when building BBHs.  On Windows with GDI,
the bounds were sometimes still slightly slightly too tight compared to the slow
method, and my bounds-contains-slow-bounds assert was triggering.  Lost steam
investigating that.  I don't think anything rendered wrong, but watch out for
the GDI bot.

[f(malita), 2015-01-28 18:10:23]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-01-28 18:11:28]

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

[commit-bot: I haz the power, 2015-01-28 18:56:09]

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