Remove subtle radial gradients

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia
P1
blocker
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: khuey, Assigned: aus)

Tracking

({memory-footprint, perf})

unspecified
2.1 S1 (1aug)
memory-footprint, perf

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v1.4 wontfix, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [c=memory p= s=2014.08.01.t u=2.0] [MemShrink:P1][systemsfe][p=3])

Attachments

(4 attachments, 1 obsolete attachment)

We have a number of radial gradients that are basically "dark gray background with a slightly lighter radial gradient".  Each one of these uses MBs of memory when rendered and BenWa tells me that they're quite slow to draw on ARM too.  We should ditch them.
blocking-b2g: --- → 2.0?
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #0)
> BenWa tells me that they're quite slow to
> draw on ARM too.  We should ditch them.

I see 2.5 MB of memory usage split between 3 gradient.png.

The current images have the radial gradient pre-generated. This consumes a lot of memory. If we were to generate them on-draw (to what I was alluding to in Kyle's comment) this would be very expensive because sqrt are expensive to compute there but we would save the memory usage. Both of these are a terrible option. Note that a linear gradient would probably be cheap enough to generate on draw.

We should consider replacing the effect with something else that is more efficient or removing the effect.

We could also experiment having a very small gradient image and stretching that. That could save us a lot of memory and preserving It -might- look fine since it's a dark and fuzzy gradient to begin with, but it might also look terrible.

Updated

4 years ago
Priority: -- → P3
Whiteboard: [MemShrink] → [MemShrink][c=memory p= s= u=]

Updated

4 years ago
blocking-b2g: 2.0? → 2.0+
Benoit, we need to know two things here - where are the radial gradient PNG files located in the file system, and where do they show up in the GUI? We need this in order to try anything suggested in comment 1...
Flags: needinfo?(bgirard)
Priority: P3 → P1

Updated

4 years ago
Severity: normal → blocker
The best way to get it is to run tools/get_about_memory.py and look at what is currently loaded under the image section and grepping to find how it comes about.
Flags: needinfo?(bgirard)
(Assignee)

Comment 5

4 years ago
I can look into this and generate some numbers. Probably the biggest gains we can get will be to remove them from the shared elements. We can let apps decide if they want to eat more memory for gradients. :)
Assignee: nobody → aus
Status: NEW → ASSIGNED
(Assignee)

Comment 6

4 years ago
The only consistent gradient used is in the system app: 0.59 MB (01.46%) -- image(app://system.gaiamobile.org/shared/style/confirm/images/ui/gradient.png)

It's always on screen. Other gradients are temporary when used by shared elements. Not sure that it's worth doing this at this time.
(Assignee)

Comment 7

4 years ago
System App Memory Usage (Main Process) w/gradients: 40.88 MB, w/o gradients: 39.74 MB.

Actually kind of a sizable chunk. This was done by simply removing the gradient from the shared 'confirm' dialog css.
(Assignee)

Comment 8

4 years ago
swilkes, if we were to genuinely remove these gradients, what would we want to replace them with? Flat colors? http://mxr.mozilla.org/gaia/find?text=&string=gradient << file list.
Flags: needinfo?(swilkes)
(Assignee)

Updated

4 years ago
Whiteboard: [MemShrink][c=memory p= s= u=] → [MemShrink][c=memory p= s= u=][systemsfe][p=]
The gradient can be kept but instead specified inline. It might need to be decoded on demand which would hurt the time to display the menu but it would stop hurting other things by forcing needless memory pressure/background processes to be killed.

Otherwise a linear CSS gradient should be fast enough to draw in CSS. But yes a flat color should draw much faster.
Does the gradient really add much visually? Sounds like the effect is very subtle. Can we try just a flat color and see if that's acceptable?
Whiteboard: [MemShrink][c=memory p= s= u=][systemsfe][p=] → [MemShrink:P1][c=memory p= s= u=][systemsfe][p=]
Target Milestone: --- → 2.1 S1 (1aug)

Comment 11

4 years ago
No one from visual design is on this bug. Adding them now. This is their call, at least in part.
Flags: needinfo?(swilkes) → needinfo?(padamczyk)
Hey Kyle, do you have examples / screenshots. If its for app backgrounds we're going to remove these soon as part of the building block updates.
Flags: needinfo?(padamczyk) → needinfo?(khuey)
I don't know where they are, but based on the paths I suspect they're in the value selector, date selector, etc.
Flags: needinfo?(khuey)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #13)
> I don't know where they are, but based on the paths I suspect they're in the
> value selector, date selector, etc.

We removed some on tarako in bug 1001455 if that can help finding them.
Immediately after startup on trunk (without even unlocking the phone) I see

app://system.gaiamobile.org/shared/style/confirm/images/ui/gradient.png
app://callscreen.gaiamobile.org/shared/style/action_menu/images/ui/gradient.png

loaded.  Each is using 0.59 MB of memory.

I find it surprising that we've triggered loading these images at all.
Aus, can you apply the patch from bug 1001455 and get screenshots for Patryk?

https://bugzilla.mozilla.org/attachment.cgi?id=8412720&action=diff
Flags: needinfo?(aus)
(Assignee)

Comment 17

4 years ago
Yep. I can do that today. Leaving NI? flag on until I've fulfilled the request. :)
(Assignee)

Comment 18

4 years ago
Created attachment 8463634 [details] [review]
Pull Request - Remove gradients from lockscreen, shared elements.
Flags: needinfo?(aus)
(Assignee)

Comment 19

4 years ago
Created attachment 8463644 [details]
Screenshots - What it looks like without said gradients.
Attachment #8463644 - Flags: ui-review?(padamczyk)
(Assignee)

Updated

4 years ago
Whiteboard: [MemShrink:P1][c=memory p= s= u=][systemsfe][p=] → [MemShrink:P1][c=memory p= s= u=][systemsfe][p=3]
Created attachment 8464009 [details]
missing.png

Looks like the button divider is missing. See what I mean in the screenshot highlighted but the magenta ellipse.
Flags: needinfo?(aus)
Comment on attachment 8463644 [details]
Screenshots - What it looks like without said gradients.

The divider is missing. 
Przemek can you also look at this attachment. Can we apply the new design?
Attachment #8463644 - Flags: ui-review?(padamczyk) → ui-review-
Flags: needinfo?(pabratowski)
(Assignee)

Comment 22

4 years ago
The divider looks really jarring against the flat color so I assumed we would want it to be removed. I can put it back and send you another screenshot but... believe me, it's nasty looking. :)

Patryk, I'm not sure which attachment you are talking about... is it the one showing the missing divider or a different 'new' design screenshot that's not attached yet?
Flags: needinfo?(aus)
Patryk, I'm also confused by your question? If you're talking about the 2.2 design for these dialogs then I think that would be too much of a change to make here.
Flags: needinfo?(pabratowski)
I think I also prefer adding back the divider tone that Patryk said is missing, maybe we can make it far more subtle?
(Assignee)

Comment 25

4 years ago
The divider is currently an image instead of a flat color. I think having a flat color to go with the rest of the panel would be more pleasing. Also to note that the divider is only present in confirm dialogs. I'll provide screenshots of one with the pattern that was there before and one with an awesome color I'll pick.
(Assignee)

Comment 26

4 years ago
Created attachment 8464243 [details]
UI/UX Review - With Previous Separator Pattern
Attachment #8464009 - Attachment is obsolete: true
Attachment #8464243 - Flags: ui-review?(padamczyk)
(Assignee)

Comment 27

4 years ago
Created attachment 8464244 [details]
UI/UX Review - With Accent Color (from our colour palette)
Attachment #8464244 - Flags: ui-review?(padamczyk)
Comment on attachment 8464244 [details]
UI/UX Review - With Accent Color (from our colour palette)

Looks good!
Attachment #8464244 - Flags: ui-review?(padamczyk) → ui-review+
Comment on attachment 8464243 [details]
UI/UX Review - With Previous Separator Pattern

The solid colour one looks MUCH better.
Attachment #8464243 - Flags: ui-review?(padamczyk) → ui-review-
(Assignee)

Comment 30

4 years ago
Awesome! Thanks Patryk. I'm going to commit this to master and 2.0 shortly. :)
(Assignee)

Comment 31

4 years ago
Commit (master): https://github.com/mozilla-b2g/gaia/commit/80d9343cdce468eee9642829e68acc9ee0e47487

Commit (v2.0): https://github.com/mozilla-b2g/gaia/commit/0bc0fb854b503ac38d9184c8325062987b464cbe

Fixed!
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-b2g-v1.4: --- → wontfix
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
Resolution: --- → FIXED

Updated

4 years ago
Whiteboard: [MemShrink:P1][c=memory p= s= u=][systemsfe][p=3] → [c=memory p= s=2014.08.01.t u=2.0] [MemShrink:P1][systemsfe][p=3]
Depends on: 1094428
You need to log in before you can comment on or make changes to this bug.