907108 - [B2G][Helix][Gaia][meijinfang]The orientation of iamge will be inverted in the edit view of gallery.

Status: VERIFIED FIXED

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

Description

[lecky]

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; aff-kingsoft-ciba; .NET4.0C; .NET4.0E)

Steps to reproduce:

1.Take a photo by camera.
2.View the photo in gallery.
3.Let the device in portrait mode, press "edit" icon to join edit view.




Actual results:

The image in edit view shall be inverted.


Expected results:

The orientation of the image in edit view should be normal.

Comment 1

[Dominic Kuo [:dkuo]]

Attachment: Screenshot

I can reproduce this, not sure what's going on need to take a look.

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

I can't reproduce it with 1.1.0hd 2013-08-28-04-22-01 build from pvt server.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

build information

gecko: 05853a4c86c59742cc0f62b198a41181672ac83d
gaia: b26e980da9819daaf915f09be7eb0b12f69ba24a
From PVT, 20130828042201

Comment 4

[Wayne Chang [:wchang]]

Do we have any progress on this?

John, if you want you can check this on my device. It needs to be a helix device.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi Wayne, 

I can't reproduce it. It needs time to find out which patch fixed it.

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

Wayne, 

I have a helix now and still can't reproduce it with the version I posted at comment 2 and comment 3.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

Lecky,

Please tell us the build version you use.

Comment 8

[Dominic Kuo [:dkuo]]

I was not using the latest build so I think John is correct, let's wait for lecky to tell us the build version that he has been testing.

Comment 9

[lecky]

We have find the cause of this issue, it's a patch of Qualcomm, not gecko or gaia.
Thanks for your help.

Comment 10

[lecky]

(In reply to John Hu [:johnhu] from comment #7)
> Lecky,

Please tell us the build version you use.

Hi John,

[Gaia commit ID] c0ea0a4943dc8d3751b07f5b5c5d3abe06364a14 
[gecko commit ID] 170f9e477571127cd40997fa2abe262ed43f0e4d
[gonk-misc commit ID]ca9192d7ef6037016d4723647d86b72f7a410633

Comment 11

[lecky]

Hi John,

I added some logs in function HwcComposer2D::PrepareLayerList() to print the variable 'mList'.
We found that the value of 'transform' had been wrong in this layer.

+++++++++++++++++++++++++++++++++
E/HWComposer(  165): hwc_prepare DBG: mList->numHwLayers[2]
E/HWComposer(  165):  compositionType = 2;
E/HWComposer(  165):  hints = 0;
E/HWComposer(  165):  flags = 0;
E/HWComposer(  165):  transform = 0;
E/HWComposer(  165):  blending = 261;
E/HWComposer(  165):  sourceCrop.left = 0;
E/HWComposer(  165):  sourceCrop.top = 0;
E/HWComposer(  165):  sourceCrop.right = 480;
E/HWComposer(  165):  sourceCrop.bottom = 800;
E/HWComposer(  165):  displayFrame.left = 0;
E/HWComposer(  165):  displayFrame.top = 0;
E/HWComposer(  165):  displayFrame.right = 480;
E/HWComposer(  165):  displayFrame.bottom = 800;
E/HWComposer(  165): hwc_prepare DBG: mList->numHwLayers[3]
E/HWComposer(  165):  compositionType = 2;
E/HWComposer(  165):  hints = 0;
E/HWComposer(  165):  flags = 0;
E/HWComposer(  165):  transform = 2;
E/HWComposer(  165):  blending = 261;
E/HWComposer(  165):  sourceCrop.left = 0;
E/HWComposer(  165):  sourceCrop.top = 0;
E/HWComposer(  165):  sourceCrop.right = 300;
E/HWComposer(  165):  sourceCrop.bottom = 363;
E/HWComposer(  165):  displayFrame.left = 15;
E/HWComposer(  165):  displayFrame.top = 90;
E/HWComposer(  165):  displayFrame.right = 465;
E/HWComposer(  165):  displayFrame.bottom = 635;
D/HWComposer(  165): Frame rendered
+++++++++++++++++++++++++++++++++++++++
you can find there is a layer with transform equal to 2, and 2 means AL_TRANSFORM_FLIP_V, I think this is why the image reversing. 
+++++++++++++++++++ 
enum { 
/* flip source image horizontally (around the vertical axis) */ 
HAL_TRANSFORM_FLIP_H = 0x01, 
/* flip source image vertically (around the horizontal axis)*/ 
HAL_TRANSFORM_FLIP_V = 0x02, 
/* rotate source image 90 degrees clockwise */ 
HAL_TRANSFORM_ROT_90 = 0x04, 
/* rotate source image 180 degrees */ 
HAL_TRANSFORM_ROT_180 = 0x03, 
/* rotate source image 270 degrees clockwise */ 
HAL_TRANSFORM_ROT_270 = 0x07, 
}; 
+++++++++++++++++++ 
So looks like Gecko/Gonk set this value.
Can you help to check it?

Comment 12

[lecky]

Attachment: launch the gallery APP-->open a picture -->click the edit button

Comment 13

[John Hu [:johnhu][:johu][:醬糊小弟]]

Cool, lecky.

I need to ask who is the best one to deal with this.

Comment 14

[Wayne Chang [:wchang]]

Lecky, John,

Can you explain what happened between comment 9 and comment 10? Why was this reopened?

Comment 15

[John Hu [:johnhu][:johu][:醬糊小弟]]

Sorry,

I have no idea about why reopened. Maybe, the new clue is found, comment 11.

Comment 16

[John Hu [:johnhu][:johu][:醬糊小弟]]

Peter, 

I heard that you know about HWComposer. Any suggestions about comment 11???

Comment 17

[lecky]

(In reply to Wayne Chang [:wchang] from comment #14)
> Lecky, John,

Can you explain what happened between comment 9 and comment
> 10? Why was this reopened?

Hi Wayne,

First we found that this issue disappeared when we removed the property 'ro.display.colorfill=1' from file \device\qcom\b2g_common\b2g_product.mk. So we thought that it was Qualcomm's responsibility.

But after debugging with Qualcomm, we found that Gecko/Gonk set the transform value to 2 and let the image revers. So we need your help to find where to set this value.

Comment 18

[Peter Chang[:pchang]]

(In reply to lecky from comment #17)
> (In reply to Wayne Chang [:wchang] from comment #14)
> > Lecky, John,
> 
> Can you explain what happened between comment 9 and comment
> > 10? Why was this reopened?
> 
> Hi Wayne,
> 
> First we found that this issue disappeared when we removed the property
> 'ro.display.colorfill=1' from file \device\qcom\b2g_common\b2g_product.mk.
> So we thought that it was Qualcomm's responsibility.
> 
> But after debugging with Qualcomm, we found that Gecko/Gonk set the
> transform value to 2 and let the image revers. So we need your help to find
> where to set this value.

Do your gecko have the following commit?

commit fb41544b355d5825737572a592e576e3d288f4d7
Author: Diego Wilson <[e-mail]>
Date:   Wed Aug 14 12:14:17 2013 -0700

    Bug 903562 - Honor horizontal and vertical reflection. r=ncameron, r=vliu, a=hd+

Comment 19

[Peter Chang[:pchang]]

lecky,

I double check this issue and found I still could reproduce it after I disable hwc.
Therefore, this issue was not related to hwcomposer.



[How to disable hwcomposer]
a. adb shell mv /system/lib/hw/hwcomposer.msm7627a.so /system/lib/hw/hwcomposer.msm7627a.so_bak
b. adb shell stop b2g
c. adb shell start b2g

Comment 20

[Peter Chang[:pchang]]

Attachment: 0001-Merge-from-Bug-788975-Orientation-Observer-uses-acce.patch

Found this issue was related to screen rotation, not related to GPU/HWC composition.

And it was fixed at bug 788975 which uses Acceleration sensor to get orientation.

Please use attached merged patch to check in your side.

Comment 21

[lecky]

(In reply to peter chang[:pchang] from comment #19)
> lecky,

I double check this issue and found I still could reproduce it after
> I disable hwc.
Therefore, this issue was not related to hwcomposer.



[How
> to disable hwcomposer]
a. adb shell mv /system/lib/hw/hwcomposer.msm7627a.so
> /system/lib/hw/hwcomposer.msm7627a.so_bak
b. adb shell stop b2g
c. adb shell
> start b2g

Hi peter,

When I disable hwc according to your method, I could not reproduce this issue on my device.

I will merge the patch you told to check it.

Comment 22

[Peter Chang[:pchang]]

> Hi peter,
> 
> When I disable hwc according to your method, I could not reproduce this
> issue on my device.
> 
> I will merge the patch you told to check it.

Don't know why you can't reproduce this problem, but I still could see this issue after HWC disabled.
Could you verify this patch with HWC enable/disable and let me know the result.

Comment 23

[lecky]

Hi peter,

I merged the patch you told at bug 788975 and I still could reproduce this issue.

I still thought that this issue was related to GPU/HWC composition.

Comment 24

[lecky]

(In reply to lecky from comment #23)
> Hi peter,

I merged the patch you told at bug 788975 and I still could
> reproduce this issue.

I still thought that this issue was related to
> GPU/HWC composition.

On this basis disabled HWC with your method, this issue cannot be reproduced.

Comment 25

[William Hsu [:whsu]]

Hi, all,

As I tested on customization build(partner provided) and newer Mozilla builds.
If I removed the "hwcomposer.msm7627a.so", the issue cannot be reproduced.
If I restored the "hwcomposer.msm7627a.so", the issue can be reproduced.

* Test Build: (Customization version - partner build)
  - Gaia:     "c545f2f862704ca5cdb6d32e03fddf711a642d35"
  - Gecko:    ""
  - BuildID:  20130813223021
  - Version:  18.0
-> Result: 
   + Remove the "hwcomposer.msm7627a.so": The image is displayed as normal.
   + Restore the "hwcomposer.msm7627a.so": The image is upside-down

* Test Build: (Mozilla build - V1.1.0 hd)
  - Gaia:     "75228cd1223c22642623e99c37f7bc8b2a355d86"
  - Gecko:    "http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/4b09975b2d7c"
  - BuildID:  20130909042204
  - Version:  18.0
-> Result: 
   + Remove the "hwcomposer.msm7627a.so": The image is displayed as normal.
   + Restore the "hwcomposer.msm7627a.so": The image is upside-down


* Test Build: (Mozilla build - Mozilla Central)
  - Gaia:     "40a1cf0e29c633a879513b261af0340b69231f7b"
  - Gecko:    "http://hg.mozilla.org/mozilla-central/rev/be1053dc223b"
  - BuildID:  20130910040201
  - Version:  26.0a1
-> Result: 
   + Remove the "hwcomposer.msm7627a.so": The image is displayed as normal.
   + Restore the "hwcomposer.msm7627a.so": The image is upside-down


--------------------------------------------------------------------------
Attaching my test steps:

* If it is a customization build,
1. Get the build from partner server and flash the image into helix device
2. Launching the Gallery app and edit a picture. ( --> Observing the result)
3. Removing the "hwcomposer.msm7627a.so" and reboot the device(/system/lib/hw)
4. Launching the Gallery app and edit a picture. ( --> Observing the result)

* If it is a Mozilla build,
1. Get the build from PVT server and flash the "Gecko" and "Gaia" into helix device.
   (So, we still used the partner provided Gonk to test the issue)
2. Launching the Gallery app and edit a picture. ( --> Observing the result)
3. Removing the "hwcomposer.msm7627a.so" and reboot the device(/system/lib/hw)
4. Launching the Gallery app and edit a picture. ( --> Observing the result)
--------------------------------------------------------------------------


Please correct me if I didn't test the correct build.

Comment 26

[William Hsu [:whsu]]

One more information, I don't know whether the following bug impact our test result.
- https://bugzilla.mozilla.org/show_bug.cgi?id=908630

Our partner solved the issue above recently but forgot to update the information.
We could use the latest customization build to do the test again.
Thank you.

Comment 27

[Peter Chang[:pchang]]

(In reply to William Hsu [:whsu] from comment #26)
> One more information, I don't know whether the following bug impact our test
> result.
> - https://bugzilla.mozilla.org/show_bug.cgi?id=908630
> 
> Our partner solved the issue above recently but forgot to update the
> information.
> We could use the latest customization build to do the test again.
> Thank you.

I thought I hit this problem during my testing.

Comment 28

[William Hsu [:whsu]]

Hi, all,

Update the status.
After I discussed this case with Peter and John this morning, we found hwcomposor impacted our test result.
If we disable hwcomposor, the problem cannot be reproduced no matter whether we fix the sensing value(Orientation problem) or not.

Doing the second testing on the different devices, I cannot reproduce this case on Leo device but I can reproduce this case on Helix device with the same Gaia patch.

So, we may address/trace this problem from Gecko layer.
Thank you.

Comment 29

[Peter Chang[:pchang]]

The image was used a canvas layer with GL-renderering, therefore, we will let this layer carry the Y-FLIP flag to GPU or HWC composition. That's why you see transform as 2 for that img layer.

But it is very strange that we don't have problem in another qualcomm device which also has the same input as helix hwcomposer.

Comment 30

[Peter Chang[:pchang]]

Attachment: hwc_debug

Attached the hwc debug patch.

For the transform flag set as 2 by FxOS, it is expected behavior because that layer is using GL-rendering. Therefore, we need composer to do the vertical flip for it.

I created a hwc debug patch which tried to reset transform back to 0 when problem happened.
And found it didn't flip vertical back on helix platform.

But I couldn't reproduce this issue with another qcom device with HWC.
Please check this issue in your HAL or kernel driver.

[How to]
a. adb shell setprop debug.gfx.settran 1   
b. try to update the screen
    ==> you will see transfrom change from 2 to 0 from logcat

Comment 31

[lecky]

Hi Peter,

Could you upload one log that is gotten from your another qualcomm device which can not reproduce this issue?

Thanks

Comment 32

[Peter Chang[:pchang]]

I tested with CrystalSkull which is a GL-rendering app.

The log I got from that qcom device.
I/Gecko   (12412):   layer 0x4413e000 transform 2 xx 1.000000 yy 1.000000 xy 0.000000 yx 0.000000 flipped 1

Comment 34

[liangxin]

(In reply to peter chang[:pchang] from comment #19)
> lecky,
> 
> I double check this issue and found I still could reproduce it after I
> disable hwc.
> Therefore, this issue was not related to hwcomposer.
> 
> 
> 
> [How to disable hwcomposer]
> a. adb shell mv /system/lib/hw/hwcomposer.msm7627a.so
> /system/lib/hw/hwcomposer.msm7627a.so_bak
> b. adb shell stop b2g
> c. adb shell start b2g

This issue does not reproduce when I disable hwcomposer follow your method。

Comment 35

[Peter Chang[:pchang]]

(In reply to peter chang[:pchang] from comment #30)
> Created attachment 804173 [details] [diff] [review]
> hwc_debug
> 
> Attached the hwc debug patch.
> 
> For the transform flag set as 2 by FxOS, it is expected behavior because
> that layer is using GL-rendering. Therefore, we need composer to do the
> vertical flip for it.
> 
> I created a hwc debug patch which tried to reset transform back to 0 when
> problem happened.
> And found it didn't flip vertical back on helix platform.
> 
> But I couldn't reproduce this issue with another qcom device with HWC.
> Please check this issue in your HAL or kernel driver.
> 

lecky, do you have any update?

Comment 36

[lecky]

Hi Peter,

Through further debugging,we still thought that the issue was in gecko/gonk layer.

We got the every frame dump by below ways.
[How to]
1/adb shell setprop debug.sf.dump.enable true 
2/adb shell setprop debug.sf.dump.png 80 
Then layers are dumped in the time-stamped location and format:
/data/sfdump.png<YYYY><MM><DD>.<HH><MM><SS>/sfdump<dump frame no.>_layer<layer no.>.png 
To stop dumping [and logging] at any instant, set adb shell setprop debug.sf.dump.png 0. 
Layer dumps are done for <no. of frames> frames rendered after the property value is changed. 
e.g., If the current debug.sf.dump value is 10, one way to trigger the next set of frame dumps is
to do adb shell setprop debug.sf.dump 11. 

From log we can see :
++++++++++++++++++++++++++++ 
01-06 01:12:18.639: E/HWComposer(157): hwc_prepare DBG: mList->numHwLayers[3] 
01-06 01:12:18.639: E/HWComposer(157): compositionType = 2; 
01-06 01:12:18.639: E/HWComposer(157): hints = 0; 
01-06 01:12:18.639: E/HWComposer(157): flags = 0; 
01-06 01:12:18.639: E/HWComposer(157): transform = 2; 
01-06 01:12:18.639: E/HWComposer(157): blending = 261; 
01-06 01:12:18.639: E/HWComposer(157): sourceCrop.left = 0; 
01-06 01:12:18.639: E/HWComposer(157): sourceCrop.top = 0; 
01-06 01:12:18.639: E/HWComposer(157): sourceCrop.right = 300; 
01-06 01:12:18.639: E/HWComposer(157): sourceCrop.bottom = 363; 
01-06 01:12:18.639: E/HWComposer(157): displayFrame.left = 15; 
01-06 01:12:18.639: E/HWComposer(157): displayFrame.top = 90; 
01-06 01:12:18.639: E/HWComposer(157): displayFrame.right = 465; 
01-06 01:12:18.639: E/HWComposer(157): displayFrame.bottom = 635; 
...... 
01-06 01:12:19.459: E/libQcomUI(157): sfdump: [png-dump-frame: 021 of 100] [???]-Composition, Layer[3] SrcBuff[320x384] SrcCrop[0l, 0t, 300r, 363b] DispFrame[15l, 90t, 465r, 635b] Composition-type = Copybit, Format = RGBA_8888, Orientation = FLIP_V, Flags = [None] 
01-06 01:12:19.929: E/libQcomUI(157): sfdump: [png-dump-frame: 021 of 100] Dumped Layer[3] to /data/sfdump.png19800106.011103/sfdump021_layer3.png: Success 
++++++++++++++++++++++++++++ 
and you can find sfdump021_layer3.png has been vertically flipped, the behavior is what should be gotten according to gecko/gonk layer setting.

Comment 37

[Peter Chang[:pchang]]

(In reply to lecky from comment #36)
> Hi Peter,
> 
> Through further debugging,we still thought that the issue was in gecko/gonk
> layer.
> 
> We got the every frame dump by below ways.
> [How to]
> 1/adb shell setprop debug.sf.dump.enable true 
> 2/adb shell setprop debug.sf.dump.png 80 
> Then layers are dumped in the time-stamped location and format:
> /data/sfdump.png<YYYY><MM><DD>.<HH><MM><SS>/sfdump<dump frame
> no.>_layer<layer no.>.png 
> To stop dumping [and logging] at any instant, set adb shell setprop
> debug.sf.dump.png 0. 
> Layer dumps are done for <no. of frames> frames rendered after the property
> value is changed. 
> e.g., If the current debug.sf.dump value is 10, one way to trigger the next
> set of frame dumps is
> to do adb shell setprop debug.sf.dump 11. 
> 
> From log we can see :
> ++++++++++++++++++++++++++++ 
> 01-06 01:12:18.639: E/HWComposer(157): hwc_prepare DBG:
> mList->numHwLayers[3] 
> 01-06 01:12:18.639: E/HWComposer(157): compositionType = 2; 
> 01-06 01:12:18.639: E/HWComposer(157): hints = 0; 
> 01-06 01:12:18.639: E/HWComposer(157): flags = 0; 
> 01-06 01:12:18.639: E/HWComposer(157): transform = 2; 

As I mentioned in comment 30, could you hack the transform from 2 to 0? And then you could see the transform behavior on helix device. I saw it was trying to flip horizontal not vertical.

> 01-06 01:12:18.639: E/HWComposer(157): blending = 261; 
> 01-06 01:12:18.639: E/HWComposer(157): sourceCrop.left = 0; 
> 01-06 01:12:18.639: E/HWComposer(157): sourceCrop.top = 0; 
> 01-06 01:12:18.639: E/HWComposer(157): sourceCrop.right = 300; 
> 01-06 01:12:18.639: E/HWComposer(157): sourceCrop.bottom = 363; 
> 01-06 01:12:18.639: E/HWComposer(157): displayFrame.left = 15; 
> 01-06 01:12:18.639: E/HWComposer(157): displayFrame.top = 90; 
> 01-06 01:12:18.639: E/HWComposer(157): displayFrame.right = 465; 
> 01-06 01:12:18.639: E/HWComposer(157): displayFrame.bottom = 635; 
> ...... 
> 01-06 01:12:19.459: E/libQcomUI(157): sfdump: [png-dump-frame: 021 of 100]
> [???]-Composition, Layer[3] SrcBuff[320x384] SrcCrop[0l, 0t, 300r, 363b]
> DispFrame[15l, 90t, 465r, 635b] Composition-type = Copybit, Format =
> RGBA_8888, Orientation = FLIP_V, Flags = [None] 
> 01-06 01:12:19.929: E/libQcomUI(157): sfdump: [png-dump-frame: 021 of 100]
> Dumped Layer[3] to /data/sfdump.png19800106.011103/sfdump021_layer3.png:
> Success 
> ++++++++++++++++++++++++++++ 
> and you can find sfdump021_layer3.png has been vertically flipped, the
> behavior is what should be gotten according to gecko/gonk layer setting.

Please provide the dump image.

Comment 38

[Peter Chang[:pchang]]

> > ++++++++++++++++++++++++++++ 
> > and you can find sfdump021_layer3.png has been vertically flipped, the
> > behavior is what should be gotten according to gecko/gonk layer setting.
> 
> Please provide the dump image.

I got your attached image by mail.


As I said in comment 29.
Before passing to HWC, the image was already flip because of using GL-renderering.
And we need to set transform as 2 for composer to flip it back. As I said, please hack it to force transform as 0 and then you will see the original image orientation. 

At the same time, please compare transform 0 and transform 2 both cases. Does it flip as vertical as expected or not?

Comment 39

[lecky]

As your said,when hack the transform from 2 to 0,it did not flip as vertical as expected.

We are trying to find the reason.

Comment 40

[lecky]

Thanks all, we have found the bug in HAL level and solved it.

Comment 41

[Wayne Chang [:wchang]]

Marking this POVB and HD+ as it needs to be fixed before release, however fixed by vendor in HAL level as comment 40.

Closing this as resolved fixed per comment 40.

Thanks Lecky!

Comment 42

[Wayne Chang [:wchang]]

Adding assignee per comment 40

Comment 43

[liangxin]

(In reply to lecky from comment #40)
> Thanks all, we have found the bug in HAL level and solved it.

Hi,do you have marged the solution or patch to firefox V1.1? can you give me the solution or patch?

Comment 44

[William Hsu [:whsu]]

Hi, Lecky,

Could you please provide the root cause?

Comment 45

[liangxin]

Hi,everyone.Is there any advance? Could you please provide the root cause?

Comment 46

[liangxin]

(In reply to lecky from comment #40)
> Thanks all, we have found the bug in HAL level and solved it.

Hi.Could you provide me the patch to this issue?

Comment 47

[Al Tsai [:atsai]]

[2013/10/21 Helix Testing]
Gaia:     c829a2042594b6c3a4899ee27979799a0f301534
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/f7c657f6d019
BuildID   20131015042201
Version   18.0

Comment 48

[William Hsu [:whsu]]

Remove NI.