Open Bug 1169176 Opened 7 years ago Updated 4 years ago

Investigate the value of external display's dpi returned by hwc

Categories

(Core :: Graphics, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

()

People

(Reporter: shelly, Unassigned)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 1 obsolete file)

When using
  int32_t values[3];
  const uint32_t attrs[] = {
      HWC_DISPLAY_WIDTH,
      HWC_DISPLAY_HEIGHT,
      HWC_DISPLAY_DPI_X,
      HWC_DISPLAY_NO_ATTRIBUTE
  };
  mHwc->getDisplayAttributes(mHwc, HWC_DISPLAY_EXTERNAL, 0, attrs, values);

Result of values[2], which is the xdpi of external display, returns 0.
Blocks: 1116089
Blocks: 1173258
No longer blocks: 1173258
In [1], we use 75 as the default value for DPI_X. I think we should just use zero as the DPI_X value if we get the zero value. I found the definition in [2] said "if the DPI for a configuration is unavailable or the HWC implementation considers it unreliable, it should set these attributes to zero." And it won't cause any problem.

[1]: https://dxr.mozilla.org/mozilla-central/source/widget/gonk/libdisplay/GonkDisplayJB.cpp#35
[2]: http://androidxref.com/4.4.2_r1/xref/hardware/libhardware/include/hardware/hwcomposer_defs.h#164
Attached patch Bug-1169176-hg.patch (obsolete) — Splinter Review
I remove the default value 75 for x dpi when we cannot get the dpi info.
Assignee: nobody → kuoe0
Attachment #8645648 - Flags: review?(sotaro.ikeda.g)
Sorry, I forgot to remove the unnecessary comment.
Attachment #8645648 - Attachment is obsolete: true
Attachment #8645648 - Flags: review?(sotaro.ikeda.g)
Attachment #8645649 - Flags: review?(sotaro.ikeda.g)
Depends on: 1138287
Comment on attachment 8645649 [details] [diff] [review]
Bug-1169176-hg.patch

Review of attachment 8645649 [details] [diff] [review]:
-----------------------------------------------------------------

Can you explain why "Xdpi == 0" is ok for gecko? IIRC, it is not acceptable for gecko when the display own a document. [2] in Comment 1 is about hwc hal. It does not talk about android. Android does not use 0 for xdpi. In HWComposer::queryDisplayProperties(), if xdpi of hwc is 0, it get default value by calling getDefaultDensity().
  http://androidxref.com/5.1.1_r6/xref/frameworks/native/services/surfaceflinger/DisplayHardware/HWComposer.cpp#317

I do not know from where 75.0 came. It seems better to align to android's implementation. Android value seems to be defined around the following.
  http://androidxref.com/5.1.1_r6/xref/frameworks/native/include/android/configuration.h#43
Attachment #8645649 - Flags: review?(sotaro.ikeda.g)
Component: General → Graphics
Product: Firefox OS → Core
I think you are right! Maybe we have to get a default value for DPI when we get a zero value for DPI. 

I have asked Henry about 75.0 for DPI. He said that it is hardcode for some displays. So, I think we should remove it and use a default value for DPI when we get zero for DPI. I'm also investigating the EDID info. I think maybe we can calculate the correct DPI with it.
Assignee: kuoe0 → nobody
Is this bug still relevant to current Connect Devices initiatives?
Whiteboard: [gfx-noted]
You need to log in before you can comment on or make changes to this bug.