Closed Bug 1010412 Opened 8 years ago Closed 8 years ago

[Flame] Wallpapers are Fuzzy

Categories

(Firefox OS Graveyard :: Gaia::Wallpaper, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: padamczyk, Assigned: pdahiya)

References

Details

Attachments

(2 files)

It appears that the builds are not grabbing the correct wallpaper sizes for Flame, so all the default wallpapers are fuzzy.

The correct wallpapers are found here:
https://github.com/mozilla-b2g/gaia/tree/master/apps/wallpaper/resources

They are identified by containing a "F" in the file name before the scale factor... ie:
FX_Leap@F1.5x.jpg

The F denotes the "F" in FWVGA.
Looks like bug 1010516 fixes most of the fuzziness, however there still seems to be a bit of scaling. See attachment in Comment 1. Also I noticed the screenshots are 480x853 not 480x854 (official FWVGA size), so not sure what is happening with that.
Depends on: 1010516
Blocks: 1016987
Hey David,
do you have idea whether we use correctly @F1.5x images? Because I can't find where we use them.
Flagging DJF to Pavel's comment #3.
Flags: needinfo?(dflanagan)
I don't actually know much about this at all, but have investigated. As far as I can from the code tell we're probably just using the 480x800 1.5x wallpapers instead of the 480x854 F1.5x wallpapers.

It appears that the F1.5x wallpaper files were introduced in bugs 949024, 950147 and 979696. I can't find any bugs, however, that modify the build script to support the "F" part.  Patryk: do you know about the history of the naming scheme here? Did you have some reason to believe that files with an "F1.5x" suffix would be automatically be used instead of the "1.5x" files on the flame?

The scripts in build/webapp-shared.js and build/webapp-zip.js both have code that deals with images and their @...x suffixes. I can't figure out how that code gets the correct multiplier value, but I can tell that it only uses the multiplier (1.5, 2.0, 2.25) and does not take screen resolution into account. It does not appear that our build system every cares about the vertical resolution of the screen.

A simple solution here might be to discard the existing 1.5x files and rename the F1.5x files to 1.5x. Then all 1.5x devices would get the 480x854 images and on 480x800 devices the top and bottom 27 pixels would be truncated. If that is okay, it would be a simple way to fix this, I think.

If it is not okay, then we need to rethink the build system.  For most of our images in most of our apps, it is only the device pixel ratio that we care about, not the screen size.  For wallpapers, however, we do care about the screen size. We've been trying to use the dpr "@x" solution to work for wallpapers, but it really can't work in the general case.  For wallpapers we need to customize based on the screen size and device form factor and we should be using a separate customization mechanism.

Needinfo Patryk to ask where the "F1.5x" suffix came from

Needinfo Yuren for his ideas on how the build system could be modified to determine the target screen resolution at build time.
Flags: needinfo?(yurenju.mozilla)
Flags: needinfo?(padamczyk)
Flags: needinfo?(dflanagan)
we don't have mechanism for @F1.5x images, so we only use @1.5x images if using GAIA_DEV_PIXELS_PER_PX=1.5 to build, and in this case we should get images with certain resolution just like David said on comment 5, but we got too many parameters for make command (and add RESOLUTION parameter sounds not a good idea), we may consider use a configration file like .mozconfig.json for build configration.

I recommand solving this issue by simple way (rename F1.5x images to 1.5) and file another follow up bug to introduce new mechanism for building with certain resolution.
Flags: needinfo?(yurenju.mozilla)
The @F1.5x came from the Taipei devs. Helen can comment further on who spec'd it.
Flags: needinfo?(padamczyk) → needinfo?(hhuang)
According Bug 949024 request that we needed 480x800 and 480x854 sizes wallpapers, so I updated 480x854 wallpapers and used @F1.5x suffix for differentiating with @1.5x wallpapers. If currently we can't automatically use "F1.5x" instead of the "1.5x" wallpapers on the flame, I agree with the simple solution.

Patryk, I would like to know if we decide to use 480x854 wallpapers for both resolution.
Flags: needinfo?(hhuang) → needinfo?(padamczyk)
David, I am okay with the simple solution: use 480x854 wallpapers for both WVGA and FWVGA. Let me know if this is how you are going to proceed (needs info me). BTW for V.2 we added new wallpapers.
Flags: needinfo?(padamczyk) → needinfo?(dflanagan)
Patryk, yes, let's just go with the taller wallpapers for both 480x800 and 480x854 screens.  Do you know which devices we have use 480x800, because we will want to test that those images look good on a real device when 27 pixels are cut off the top and bottom.

Punam: can you take this bug?
Flags: needinfo?(pdahiya)
Flags: needinfo?(padamczyk)
Flags: needinfo?(dflanagan)
David, I will take it up and discard the existing 1.5x wallpapers and rename the F1.5x files to 1.5x as fix. Thanks
Flags: needinfo?(pdahiya)
Assignee: nobody → pdahiya
480x800 is used on the Helix which started to ship with v.1.1.
Flags: needinfo?(padamczyk)
Hi Patryk
Please review attached pull request which is using taller wallpapers 400x854 for 1.5x. PR is tested on flame (408x854) but need to be tested on a 400x800 device. Thanks
Attachment #8439612 - Flags: review?(padamczyk)
Looks like it works fine on a Helix. However are we down sampling these wallpapers in any way? I see a lot of digital artifacts in them compared to those on the flame.
Flags: needinfo?(pdahiya)
Comment on attachment 8439612 [details] [review]
PR with fix of Bug 1010412

minused based on visible digital artifacts
Attachment #8439612 - Flags: review?(padamczyk) → review-
(In reply to Patryk Adamczyk [:patryk] UX from comment #14)
> Looks like it works fine on a Helix. However are we down sampling these
> wallpapers in any way? I see a lot of digital artifacts in them compared to
> those on the flame.

Hi Patryk,
It will help to debug if you can provide helix build details on which attached patch is tested and a screen shot of digital artifacts seen in helix vs flame. We are down sampling to display smaller images for wallpaper thumbnails list view in 1.4, 2.0 and m-c. 

Also, while installing the patch on helix, did the make install-gaia had GAIA_DEV_PIXELS_PER_PX option specified (make install-gaia GAIA_DEV_PIXELS_PER_PX=1.5), so that it picks correct wallpaper size. Thanks
Flags: needinfo?(pdahiya) → needinfo?(padamczyk)
Comment on attachment 8439612 [details] [review]
PR with fix of Bug 1010412

Reviewed it again as per your comment. Looks good now. Ship it :)
Attachment #8439612 - Flags: review- → review+
Flags: needinfo?(padamczyk)
Thanks Patryk for review. Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/781a9b10a172fef13ddea8ebb9c8d77e6150e931
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.