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

NEW
Unassigned

Status

()

Core
Graphics
P3
normal
3 years ago
4 months ago

People

(Reporter: shelly, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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.
(Reporter)

Updated

3 years ago
Blocks: 1116089
(Reporter)

Updated

3 years ago
Blocks: 1173258
(Reporter)

Updated

3 years ago
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
Created attachment 8645648 [details] [diff] [review]
Bug-1169176-hg.patch

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)
Created attachment 8645649 [details] [diff] [review]
Bug-1169176-hg.patch

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)

Updated

2 years ago
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

Updated

2 years ago
Attachment #8645649 - Flags: review?(sotaro.ikeda.g)

Updated

2 years ago
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]
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.