46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
673.51 KB, application/zip
45.98 KB, image/png
39.44 KB, image/png
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.
4 years ago
blocking-b2g: --- → 2.0?
(In reply to Kyle Huey [:khuey] (firstname.lastname@example.org) 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.
Priority: -- → P3
Whiteboard: [MemShrink] → [MemShrink][c=memory p= s= u=]
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...
Priority: P3 → P1
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.
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
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.
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.
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.
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=]
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.
(In reply to Kyle Huey [:khuey] (email@example.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
Yep. I can do that today. Leaving NI? flag on until I've fulfilled the request. :)
Created attachment 8463634 [details] [review] Pull Request - Remove gradients from lockscreen, shared elements.
Created attachment 8463644 [details] Screenshots - What it looks like without said gradients.
Attachment #8463644 - Flags: ui-review?(padamczyk)
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.
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-
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?
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.
I think I also prefer adding back the divider tone that Patryk said is missing, maybe we can make it far more subtle?
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.
Created attachment 8464243 [details] UI/UX Review - With Previous Separator Pattern
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-
Awesome! Thanks Patryk. I'm going to commit this to master and 2.0 shortly. :)
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
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.