Closed Bug 884975 Opened 7 years ago Closed 7 years ago

[MMS] Apply 1.5x Res Version of the Images

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

x86
macOS
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

VERIFIED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: pla, Assigned: steveck)

Details

(Whiteboard: [LeoVB+])

Attachments

(2 files, 1 obsolete file)

Attached file @2x Res assets for MMS
This is an addition to the bug 878042 to add the @2x resolution version of the assets.
Assignee: nobody → kaze
Follow-up of a leo+ bug.
blocking-b2g: --- → leo?
Is this for WVGA?  If so, we want this to be hd+ not leo+
Flags: needinfo?(pla)
I am not sure - asking Vicky, who requested this.
Flags: needinfo?(pla) → needinfo?(vpg)
No, it is for the bigger screen devices, all assets in the UI are produced in 1x and 2x.
Flags: needinfo?(vpg)
We think this is only necessary for the hd branch.
blocking-b2g: leo? → hd+
sorry, only necessary for leo+ 1.1 based upon comment 4 (NOT hd)
blocking-b2g: hd+ → leo+
kaze any updates here?
Flags: needinfo?(kaze)
Attached file link to pull request (obsolete) —
https://github.com/mozilla-b2g/gaia/pull/10710

I don’t have any HIDPI device to test, feel free to send me a Peak. :-p
Attachment #769518 - Flags: review?(felash)
Flags: needinfo?(kaze)
Comment on attachment 769518 [details]
link to pull request

Alexandre, would you please test this patch on your Peak and confirm it works in HIDPI mode?
Attachment #769518 - Flags: feedback?(lissyx+mozillians)
Comment on attachment 769518 [details]
link to pull request

I don't like this approach; using rem in background-position will break as soon as the default font-size change.
Attachment #769518 - Flags: review?(felash) → review-
As per discussion on the pull request, un-assigning and handing over to the Taipei team.

Fred, I saw you worked on a hi-dpi patch yesterday, can you please look at this bug or assign it to someone else in your team?
Assignee: kaze → gasolin
Sure, I'll talk with steve(he is working on 1.5x image at bug 881155) and help do the following fix.
Comment on attachment 769518 [details]
link to pull request

talked with steve and we'd prefer land this and fixed some background-size issue at bug 881155

And we are curious about why 2x image issue is a leo+ bug?
Attachment #769518 - Flags: review?(schung)
Attachment #769518 - Flags: review?
Attachment #769518 - Flags: review-
I actually think it's a hd+ bug, you're right !
blocking-b2g: leo+ → hd?
hd+ing as required.
blocking-b2g: hd? → hd+
I have a comment on https://github.com/mozilla-b2g/gaia/pull/10710#issuecomment-20407695.
Hi Julien, Using the rem or px both works for me and I think it should not break the css sprite at least in v1.1hd. We can continue the hd work after landing this patch. What do you think?
There is no need to apply 2X image got v1.1 leo branch, even v1.1 hd branch will only required for 1.5X image. I will change the title to 1.5X image.

In this bug, I will add all the 1.5X image assets with some css fixing and it will uplift to v1.1 HD
In bug 881155, it will be hd-only patch for the remaining missing css changes in hd branch.

Hi Peter/Victoria, Could you please also provide the 1.5X images for v1.1hd branch? We already convert existing 2X images to 1.5X, but some images is still missing in the master. except for the placeholder image, we still need:

images/paperclip-button.png
images/add_contact.png
images/icons/deliveredtick.png
Flags: needinfo?(vpg)
Flags: needinfo?(pla)
Flags: needinfo?
Flags: needinfo?
Summary: [MMS] Apply 2x Res Version of the Images → [MMS] Apply 1.5x Res Version of the Images
Hi Steve, 

I've emailed you the 3 outstanding icons @1.5x. Let me know if you need anything else.
Flags: needinfo?(vpg)
Attached file Link to github
Hi Julien, I add 1.5X image assets with some css fix. 2X images is not necessary for master and hd now, thanks.
Attachment #771960 - Flags: review?(felash)
Attachment #769518 - Attachment is obsolete: true
Attachment #769518 - Flags: review?(schung)
Attachment #769518 - Flags: review?
Attachment #769518 - Flags: feedback?(lissyx+mozillians)
Comment on attachment 771960 [details]
Link to github

r=me

thanks !
Attachment #771960 - Flags: review?(felash) → review+
Land in master : 566dc43df6ebb19669471557093e12c26ccb932c
        v1.1hd : 5a83b21cb25ab012b5f468e483d323cb56e04761
Assignee: gasolin → schung
Flags: needinfo?(pla)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Changing to leo+ in order to dupe bug 889805, the same patch should be applied.

jhford: can you uplift this to v1-train also? thanks.
blocking-b2g: hd+ → leo+
Flags: needinfo?(jhford)
Duplicate of this bug: 889805
v1-train: 2d7498b0a696fa902239fc803542dab9b8ef5d4a
Flags: needinfo?(jhford)
Icon for video attachments has the correct size.
Verified with unagi v1-train 07/16 build:
Gecko-ea7cfd4
Gaia-c8f432c
Status: RESOLVED → VERIFIED
Since 1.5x assets are included in this patch, I don't think it is a good idea to uplift this patch to v1-train.

For now, the build script in v1-train could not handle the 1.5x assets with GAIA_DEV_PIXELS_PER_PX (only in master and hd branch).
So it would cause these 1.5x assets wrapped up in the app package, while they are unnecessary for v1-train.

Steve,

Do you think the above concern is reasonable?
Thanks.
Flags: needinfo?(schung)
(In reply to Rudy Lu [:rudyl] from comment #26)
> Since 1.5x assets are included in this patch, I don't think it is a good
> idea to uplift this patch to v1-train.
> 
> For now, the build script in v1-train could not handle the 1.5x assets with
> GAIA_DEV_PIXELS_PER_PX (only in master and hd branch).
> So it would cause these 1.5x assets wrapped up in the app package, while
> they are unnecessary for v1-train.
> 
> Steve,
> 
> Do you think the above concern is reasonable?
> Thanks.

Just check the build script and GAIA_DEV_PIXELS_PER_PX will not be considered in v1-train just like you said. Apologize for my negligence and I will fire another follow up patch only for removing the image assets. Thanks for the notice!
Flags: needinfo?(schung)
v1.1.0hd: 2d7498b0a696fa902239fc803542dab9b8ef5d4a
Whiteboard: [LeoVB+]
You need to log in before you can comment on or make changes to this bug.