The default bug view has changed. See this FAQ.

Vertical/Horizontal scrollbars missing with new compositor

VERIFIED FIXED

Status

()

Firefox for Android
General
P2
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: kats)

Tracking

({regression, uiwanted})

unspecified
ARM
Android
regression, uiwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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)
(Reporter)

Updated

5 years ago
Keywords: regression

Updated

5 years ago
Assignee: nobody → kgupta
Priority: -- → P2
Created attachment 574673 [details] [diff] [review]
(1/2) Back out the XUL scrollbar patch
Attachment #574673 - Flags: review?(mark.finkle)
Created attachment 574674 [details] [diff] [review]
(2/2) (WIP) Amazing rectangular scrollbars

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+
Created attachment 574680 [details]
Screenshot of current ugly scrollbars

This is what they look like with the first iteration of my patch.
Keywords: uiwanted
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+
Created attachment 575864 [details] [diff] [review]
(2/2) Amazing rectangular scrollbars (v2)

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
Last Resolved: 5 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.

Comment 13

5 years ago
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?

Updated

5 years ago
Blocks: 704784
(Reporter)

Comment 14

5 years ago
Samsung Nexus S (Android 4.0.1)
20111123040207
http://hg.mozilla.org/projects/birch/rev/cd5725c23a13
Status: RESOLVED → VERIFIED
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.