[TextBlob] Permit drawing selected text from cache, if possible.

[TextBlob] Permit drawing selected text from cache, if possible.

If the entire text is selected, then we can safely share the text blob
with the unselected version.

BUG=377845
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184273

[jbroman, 2014-10-23 14:35:18]

jbroman@chromium.org changed reviewers:
+ fmalita@chromium.org

[jbroman, 2014-10-23 14:35:19]

Not sure whether is worthwhile, but it should be safe for us to use the cache to
paint selected text in the case where all the text is selected (as we're
painting the full set of glyphs there, just possibly in a different colour).

What do you think?

[f(malita), 2014-10-23 14:45:44]

On 2014/10/23 14:35:19, jbroman wrote:
> Not sure whether is worthwhile, but it should be safe for us to use the cache
to
> paint selected text in the case where all the text is selected (as we're
> painting the full set of glyphs there, just possibly in a different colour).
> 
> What do you think?

I like the idea, but it seems a bit dangerous/tricky: looks like selectionStyle
can be different from textStyle, but we're caching the result using the same
key. I think we need to extend the key to also cover the used style in order to
be safe (at which point I'm not sure this is still worth it).

[f(malita), 2014-10-23 14:47:14]

On 2014/10/23 14:45:44, Florin Malita wrote:
> On 2014/10/23 14:35:19, jbroman wrote:
> > Not sure whether is worthwhile, but it should be safe for us to use the
cache
> to
> > paint selected text in the case where all the text is selected (as we're
> > painting the full set of glyphs there, just possibly in a different colour).
> > 
> > What do you think?
> 
> I like the idea, but it seems a bit dangerous/tricky: looks like
selectionStyle
> can be different from textStyle, but we're caching the result using the same
> key. I think we need to extend the key to also cover the used style in order
to
> be safe (at which point I'm not sure this is still worth it).

Actually, I take that back: I think none of TextPainter::Style attributes are
captured in the blob, so this should be safe. LGTM!

[jbroman, 2014-10-23 14:53:34]

On 2014/10/23 14:47:14, Florin Malita wrote:
> On 2014/10/23 14:45:44, Florin Malita wrote:
> > On 2014/10/23 14:35:19, jbroman wrote:
> > > Not sure whether is worthwhile, but it should be safe for us to use the
> cache
> > to
> > > paint selected text in the case where all the text is selected (as we're
> > > painting the full set of glyphs there, just possibly in a different
colour).
> > > 
> > > What do you think?
> > 
> > I like the idea, but it seems a bit dangerous/tricky: looks like
> selectionStyle
> > can be different from textStyle, but we're caching the result using the same
> > key. I think we need to extend the key to also cover the used style in order
> to
> > be safe (at which point I'm not sure this is still worth it).
> 
> Actually, I take that back: I think none of TextPainter::Style attributes are
> captured in the blob, so this should be safe. LGTM!

Indeed the only things that can vary are colours and shadows. :)

(And thank goodness. Could you imagine if we let you change the font size when
text is selected or something?)

[jbroman, 2014-10-23 14:53:38]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2014-10-23 14:54:49]

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

[commit-bot: I haz the power, 2014-10-23 14:59:22]

Committed patchset #1 (id:1) as 184273