1121648 - Remove code that uses #-moz-samplesize to downsample images

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[David Flanagan [:djf]]

Bug 1120056 will be removing support for the #-moz-samplesize media fragment, because bug 1045929 has implemented an alternative in gecko where images are not decoded until they are drawn and are decoded at the size at which they are drawn.

In theory, everything in Gaia will continue to work correctly when these bugs land, but:

1) We should remove the now useless #-moz-samplesize code anyway

2) There is code (at least in shared/js/media/media_frame.js) that we should optimize to take advantage of this new Gecko feature.  (In media_frame.js, the code for zooming in can be greatly simplified because Gecko will now automatically re-decode the image when we resize it and we don't have to do this manually anymore.)

3) We should check code throughout gaia that does things like thumbnail creation. If there is code that draws an image at small size and then draws it at larger size, it will probably be more efficient to turn that around and draw it larger first and then smaller so that it only has to be decoded once.

Code that should be reviewed and patched (or just removed) for this change includes:

shared/js/media/media_frame.js
shared/js/media/crop_resize_rotate.js
shared/js/media/downsample.js
shared/js/image_utils.js

Also, any files in the Gallery, Wallpaper and SMS apps that use the Downsample class or the ImageUtils.Downsample class.

Comment 1

[David Flanagan [:djf]]

Marking this bug as depending on 1045929, but not marking it as blocking 1120056 because Seth thinks we will be able to remove #-moz-samplesize without breaking anything. If that is true, then this bug is just for optimzation to take best advantage of the new feature.

Comment 2

[David Flanagan [:djf]]

I don't think that bug 1045929 is going to be uplifted into v2.2, but if it is, then this bug should become a high priority.

Comment 3

[David Flanagan [:djf]]

Needinfo Hema just so she knows about this chunk of work we'll have to do. This new gecko feature sounds like a real win, but it did take me by surprise.

Comment 4

[David Flanagan [:djf]]

I was wrong in comment 2, Seth says that he is hoping to uplift bug 1045929 into v2.2. If that happens we'll need to be on the lookout for image-related regressions in all of the media apps and will want to really test the gallery app carefully.

Needinfo No-jun so that this is on his radar.

Comment 5

[No-Jun Park [:njpark]]

marking this bug to be verified against 2.2.  
Note to testers: A comprehensive sweep of gallery tests should be done to check for any regressions.

Comment 6

[No-Jun Park [:njpark]]

So just to be clear, this bug is going to 2.2 right?  Then should this bug be marked so it'll go through the 2.2 bug approval process?

Comment 7

[David Flanagan [:djf]]

No-Jun,

The Gecko developer told me he "hopes to" get it uplifted to 2.2. It has not been uplifted yet (actually I think it hasn't landed on master yet).  I suspect that what would be most needed is to do a verification pass on master once the gekco feature does land there, to help inform the decision about uplifting it.

It is really an awesome gecko feature that should save a lot of memory for websites, so I'm in favor of uplifting the gecko patch if it is stable enough, even though it will mean more immediate work for me in this bug if it is uplifted.

In any case, the verifyme keyword is premature since the gecko change has not even landed on master yet. (It landed and was backed out, so I think it is very close).  So removing that for now.  And setting needinfo again just to be sure that you see this clarification.

Comment 8

[No-Jun Park [:njpark]]

Aha, so I should check first in master when it lands there, for some reason I thought bug 1045929 is ready to be uplifted into 2.2 any day.  I added myself in the cc list so when it lands in master, we can do a sanity check on gallery app.  Thanks!

Comment 9

[Hema Koka [:hema]]

Thanks David for letting me know.

Comment 10

[David Flanagan [:djf]]

I'm going to make this a meta bug and file individual bugs for the various parts of Gaia that still use #-moz-samplesize.

First off, bug 1140619 is the existing bug that I can use for cleaning up media_frame.js. I have that as a blocker, but am going to change it so that it is a dependency.

Comment 11

[David Flanagan [:djf]]

Bug 1140619 is for cleaning up media_frame.js.

Bug 1173516 is for cleaning up crop_resize_rotate.js and downsample.js and the apps that use them.

Bug 1173517 is for cleaning up image_utils.js and the apps that use it.

Comment 12

[David Flanagan [:djf]]

I just discovered that downsample-during-decode is not working in desktop Nightly or in yesterday's nightly build for Flame. So, for now, we still need the #-moz-samplesize code on master.s  See https://bugzilla.mozilla.org/show_bug.cgi?id=1140619#c29

Comment 13

[David Flanagan [:djf]]

Seth needs to get bug 1151359 fixed before I can remove #-moz-samplesize

Comment 14

[David Flanagan [:djf]]

1140619 was closed because the symptoms were not appearing anymore, so I've filed a new bug 1206206 for removing #-moz-samplesize from MediaFrame

Comment 15

[David Flanagan [:djf]]

My investigations in bug 1206206 today indicate that downsample-during-decode is not working. If that is true, then we won't be able to remove #-moz-samplesize yet.

I've asked Seth Fowler for clarification.