Using FloatRect in SimpleShaper and remove GlyphBounds

Use FloatRect in SimpleShaper and remove GlyphBounds.

GlyphBounds is only used in SimpleShaper. 
Removing GlyphBounds class and making consistency between SimpleShapper and 
HarfbuzzShaper code.

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

[h.joshi, 2015-01-22 06:55:24]

h.joshi@samsung.com changed reviewers:
+ dominik.rottsches@intel.com, eae@chromium.org

[h.joshi, 2015-01-22 06:57:44]

Pls review.
Windows bots are failing in different test case,
tried 3 times and all times failing test case is 
different which is not linked to above changes.

[eae, 2015-01-22 14:47:45]

I like the change to use a FloatRect instead of the one-off class, always
computing it though might be a perf hit and I'm less sure about that aspect of
the change. This is very performance sensitive.

[jbroman, 2015-01-22 15:21:21]

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

[jbroman, 2015-01-22 15:21:22]

I don't believe this is correct for RTL. In that case, we shape as though the
advances were LTR and then reverse.

In that case these bounds will be incorrect.

[h.joshi, 2015-01-22 18:10:58]

@Emil: We can change the implementation and pass pointer of FlaotRect
and then use the same to avoid always computing.

@jbroman: Not able to get comment. While testing patch, printed values and they
seemed
correct to me. Pls suggest some test case or some thing which can help
to test this further.

[h.joshi, 2015-01-23 08:33:05]

On 2015/01/22 18:10:58, h.joshi wrote:
> @Emil: We can change the implementation and pass pointer of FlaotRect
> and then use the same to avoid always computing.
> 
> @jbroman: Not able to get comment. While testing patch, printed values and
they
> seemed
> correct to me. Pls suggest some test case or some thing which can help
> to test this further.

[h.joshi, 2015-01-23 08:36:12]

last comment went without any thing added, sorry for that.

@Emil: Made changes to code to avoid always computing.

@jbroman: I re-checked the code and not sure why current changes will affect
in RTL path. Old code to calculate glyph bound was already present and in this
patch we are using FloatRect instead of the one-off class.

[h.joshi, 2015-01-29 12:36:40]

@jbroman: We have following code flow for reversing:
    if (runInfo.run.rtl()) {
        // Glyphs are shaped & stored in RTL advance order - reverse them for
LTR drawing.
        shaper.advance(runInfo.run.length());
        glyphBuffer.reverseForSimpleRTL(width, shaper.runWidthSoFar());
    }
It does not seems, changes in this patch will affect this behavior.

Maybe I am missing some part so please suggest correct way.

[jbroman, 2015-02-02 15:47:18]

Hmm, it's possible it isn't making things any worse for RTL. OK, then, I won't
block this CL.

[eae, 2015-02-02 21:45:46]

LGTM

Thank you!

[eae, 2015-02-02 21:45:52]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2015-02-02 21:46:45]

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

[commit-bot: I haz the power, 2015-02-02 23:33:20]

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

[commit-bot: I haz the power, 2015-02-02 23:33:21]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/48883)

[h.joshi, 2015-02-03 05:05:41]

Thank you Email and jbroman.

[h.joshi, 2015-02-03 05:05:59]

Thank you Emil and jbroman.

[h.joshi, 2015-02-03 05:14:35]

The CQ bit was checked by h.joshi@samsung.com

[commit-bot: I haz the power, 2015-02-03 05:15:21]

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

[commit-bot: I haz the power, 2015-02-03 06:06:03]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189386