Closed
Bug 1039631
Opened 10 years ago
Closed 10 years ago
Remove subtle radial gradients
Categories
(Firefox OS Graveyard :: Gaia, defect, P1)
Firefox OS Graveyard
Gaia
Tracking
(blocking-b2g:2.0+, b2g-v1.4 wontfix, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: khuey, Assigned: aus)
References
Details
(Keywords: memory-footprint, perf, Whiteboard: [c=memory p= s=2014.08.01.t u=2.0] [MemShrink:P1][systemsfe][p=3])
Attachments
(4 files, 1 obsolete file)
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
application/zip
|
padamczyk
:
ui-review-
|
Details |
(deleted),
image/png
|
padamczyk
:
ui-review-
|
Details |
(deleted),
image/png
|
padamczyk
:
ui-review+
|
Details |
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.
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 2.0?
Comment 1•10 years ago
|
||
(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•10 years ago
|
Priority: -- → P3
Whiteboard: [MemShrink] → [MemShrink][c=memory p= s= u=]
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 2•10 years ago
|
||
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•10 years ago
|
Severity: normal → blocker
Comment 3•10 years ago
|
||
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)
Reporter | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 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•10 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•10 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•10 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•10 years ago
|
Whiteboard: [MemShrink][c=memory p= s= u=] → [MemShrink][c=memory p= s= u=][systemsfe][p=]
Comment 9•10 years ago
|
||
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.
Comment 10•10 years ago
|
||
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=]
Updated•10 years ago
|
Target Milestone: --- → 2.1 S1 (1aug)
Comment 11•10 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)
Comment 12•10 years ago
|
||
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)
Reporter | ||
Comment 13•10 years ago
|
||
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)
Comment 14•10 years ago
|
||
(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.
Reporter | ||
Comment 15•10 years ago
|
||
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.
Comment 16•10 years ago
|
||
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•10 years ago
|
||
Yep. I can do that today. Leaving NI? flag on until I've fulfilled the request. :)
Assignee | ||
Comment 18•10 years ago
|
||
Flags: needinfo?(aus)
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8463644 -
Flags: ui-review?(padamczyk)
Assignee | ||
Updated•10 years ago
|
Whiteboard: [MemShrink:P1][c=memory p= s= u=][systemsfe][p=] → [MemShrink:P1][c=memory p= s= u=][systemsfe][p=3]
Comment 20•10 years ago
|
||
Looks like the button divider is missing. See what I mean in the screenshot highlighted but the magenta ellipse.
Flags: needinfo?(aus)
Comment 21•10 years ago
|
||
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•10 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)
Comment 23•10 years ago
|
||
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)
Comment 24•10 years ago
|
||
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•10 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•10 years ago
|
||
Attachment #8464009 -
Attachment is obsolete: true
Attachment #8464243 -
Flags: ui-review?(padamczyk)
Assignee | ||
Comment 27•10 years ago
|
||
Attachment #8464244 -
Flags: ui-review?(padamczyk)
Comment 28•10 years ago
|
||
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 29•10 years ago
|
||
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•10 years ago
|
||
Awesome! Thanks Patryk. I'm going to commit this to master and 2.0 shortly. :)
Assignee | ||
Comment 31•10 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
Closed: 10 years ago
status-b2g-v1.4:
--- → wontfix
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Resolution: --- → FIXED
Updated•10 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]
You need to log in
before you can comment on or make changes to this bug.
Description
•