702416 - [layers] Replace homebrew classes with Android ones

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Kartikaya Gupta (email:[e-mail])]

The IntPoint, FloatPoint, IntRect, and FloatRect classes should be replaced with the Android-provided Point, PointF, Rect, and RectF ones respectively.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: (1/4) Replace intpoint

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: (2/4) Replace floatpoint

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: (3/4) Replace IntRect

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: (4/4) Replace FloatRect

Comment 5

[Patrick Walton (:pcwalton)]

Comment on attachment 574397 [details] [diff] [review]
(1/4) Replace intpoint

Review of attachment 574397 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 6

[Patrick Walton (:pcwalton)]

Comment on attachment 574398 [details] [diff] [review]
(2/4) Replace floatpoint

Review of attachment 574398 [details] [diff] [review]:
-----------------------------------------------------------------

r+

Comment 7

[Patrick Walton (:pcwalton)]

Comment on attachment 574399 [details] [diff] [review]
(3/4) Replace IntRect

Review of attachment 574399 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment

::: embedding/android/GeckoApp.java
@@ -748,4 +748,4 @@
> >                  });
> >                  connectGeckoLayerClient();
> >              } else if (event.equals("PanZoom:Ack")) {
> > -                final IntRect rect = new IntRect(message.getJSONObject("rect"));
> > +                Rect rect = RectUtils.create(message.getJSONObject("rect"));

Standard Java style prefers "createInstance", but that's gratuitously long, so let's leave it as create.

::: embedding/android/gfx/TileLayer.java
@@ +58,5 @@
>      private boolean mRepeat;
>      private IntSize mSize;
>      private int[] mTextureIDs;
>  
> +    private Rect mTextureUploadRect;

I'd add a comment here saying that it's not safe to hand this rect out, since it may be mutated (via the union line below) and that would surprise callers.

Comment 8

[Patrick Walton (:pcwalton)]

Comment on attachment 574400 [details] [diff] [review]
(4/4) Replace FloatRect

Review of attachment 574400 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: embedding/android/ui/ViewportController.java
@@ +92,2 @@
>          float zoomFactor = (float)layerPageSize.width / (float)mPageSize.width;
> +        return RectUtils.scale(layerVisibleRect, 1.0f / zoomFactor);

scaleAll() adjusts X and Y as well as width and height. (Note that I'm going to be throwing a lot of this code away, so this doesn't really matter.)

Comment 9

[Kartikaya Gupta (email:[e-mail])]

http://hg.mozilla.org/projects/birch/rev/8d005019346d
http://hg.mozilla.org/projects/birch/rev/f713a9e71b50
http://hg.mozilla.org/projects/birch/rev/22f3cf68358c
http://hg.mozilla.org/projects/birch/rev/c2d1751fe274