Closed Bug 701351 Opened 13 years ago Closed 13 years ago

Vertical/Horizontal scrollbars missing with new compositor

Categories

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

ARM
Android
defect

Tracking

(firefox11 fixed, fennec11+)

VERIFIED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: aaronmt, Assigned: kats)

References

Details

(Keywords: regression, uiwanted)

Attachments

(3 files, 1 obsolete file)

Regression due to the landing of: http://hg.mozilla.org/projects/birch/rev/eaf778e88070

--
20111110040927
http://hg.mozilla.org/projects/birch/rev/1cf9e4b19819
Samsung Galaxy SII (Android 2.3.3)
Keywords: regression
Assignee: nobody → kgupta
Priority: -- → P2
Not marking this for review yet as they are really ugly and should probably use images instead of an ugly translucent black box.
Attachment #574673 - Flags: review?(mark.finkle) → review+
This is what they look like with the first iteration of my patch.
Better than nothing. I'd get them reviewed/landed and see how they perform.
Attachment #574674 - Flags: review?(pwalton)
Comment on attachment 574674 [details] [diff] [review]
(2/2) (WIP) Amazing rectangular scrollbars

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

Depending on how UX wants this to look, these might end up being NinePatchTileLayers instead. But this is fine for now.

::: embedding/android/gfx/LayerRenderer.java
@@ +139,5 @@
>  
> +        gl.glEnable(GL10.GL_BLEND);
> +
> +        /* Draw the vertical scrollbar */
> +        if (pageRect.height() > screenSize.height) {

Factor this stuff out into a separate private method, perhaps? I don't want onDrawFrame() to get too big.
Attachment #574674 - Flags: review?(pwalton) → review+
Update scrollbar patch - it's now rebased to birch tip, moved the heavy code into helper methods in ScrollbarLayer.java, and changed the BAR_SIZE to 8 so that it's a power of 2 and transparency works on pre-honeycomb as well.

Adding pcwalton for review again since it feels like there were significant changes.
Attachment #574674 - Attachment is obsolete: true
Attachment #575864 - Flags: review?(pwalton)
Comment on attachment 575864 [details] [diff] [review]
(2/2) Amazing rectangular scrollbars (v2)

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

r=me with nits addressed.

::: mobile/android/base/gfx/ScrollbarLayer.java
@@ +71,5 @@
> +
> +    void drawVertical(GL10 gl, IntSize screenSize, Rect pageRect) {
> +        float barStart = (float)screenSize.height * (float)(0 - pageRect.top) / pageRect.height();
> +        float barEnd = (float)screenSize.height * (float)(screenSize.height - pageRect.top) / pageRect.height();
> +        float scale = Math.max(1.0f, (float)(barEnd - barStart) / BAR_SIZE);

barEnd and barStart are both floats already, no need to cast.

@@ +81,5 @@
> +
> +    void drawHorizontal(GL10 gl, IntSize screenSize, Rect pageRect) {
> +        float barStart = (float)screenSize.width * (float)(0 - pageRect.left) / pageRect.width();
> +        float barEnd = (float)screenSize.width * (float)(screenSize.width - pageRect.left) / pageRect.width();
> +        float scale = Math.max(1.0f, (float)(barEnd - barStart) / BAR_SIZE);

Ditto.
Attachment #575864 - Flags: review?(pwalton) → review+
Landed blocky scrollbars. Leaving open for UX feedback and nicer-looking scrollbars.

https://hg.mozilla.org/projects/birch/rev/d099c3df6866
https://hg.mozilla.org/projects/birch/rev/77e488af39ff
Status: NEW → ASSIGNED
(In reply to Kartikaya Gupta (:kats) from comment #8)
> Landed blocky scrollbars. Leaving open for UX feedback and nicer-looking
> scrollbars.
> 
> https://hg.mozilla.org/projects/birch/rev/d099c3df6866
> https://hg.mozilla.org/projects/birch/rev/77e488af39ff

I think it's better to open a new bug, when such feedback is forth coming. CC'ing Madhava and Ian. If they have thoughts about changes, they can file a bug with designs.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
we are sure that this didn't regress panning performance?  kats? patrick?
The FPS is still stable at 60, and the panning performance seems to be about the same. The repaints happen at less than 60 FPS anyway so this change shouldn't affect any existing bottlenecks.
We need the scrollbars to fade away after panning is finished. Finger down == scrollbars viisble, Finger up == scrollbars fade away.
Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:11.0a1) Gecko/20111122 Firefox/11.0a1 Fennec/11.0a1 - Native build
Device: HTC Desire Z
OS: Android 2.3

Horizontal and vertical scrollbars are displayed, but they are not hidden when panning finished. Should we open this bug or file a new bug for the "hide scrollbars" issue?
Blocks: 704784
Samsung Nexus S (Android 4.0.1)
20111123040207
http://hg.mozilla.org/projects/birch/rev/cd5725c23a13
Status: RESOLVED → VERIFIED
tracking-fennec: --- → 11+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: