Skip to content

Fix: the screen size error.#626

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
GongHeng2017:202603261721-master-fix
Mar 26, 2026
Merged

Fix: the screen size error.#626
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
GongHeng2017:202603261721-master-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown
Contributor

@GongHeng2017 GongHeng2017 commented Mar 26, 2026

-- Get all screen size and judge it by base screen size.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-354275.html

Summary by Sourcery

Improve EDID screen size parsing to select the most accurate dimensions based on detailed timing descriptors and base size fields.

Bug Fixes:

  • Correct screen size selection by comparing all detailed timing descriptor sizes against the EDID base screen size and falling back appropriately.

Enhancements:

  • Introduce reusable parsing of detailed timing descriptors into a structured list for future screen-related calculations.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Mar 26, 2026

Reviewer's Guide

Refactors EDID screen size parsing to compute screen dimensions from all Detailed Timing Descriptors (DTDs), choose the descriptor closest to the base 15H/16H size, and fall back to the base size when no close match exists, introducing reusable DTD parsing helpers and a DTD size info struct.

Class diagram for updated EDIDParser and DTDSizeInfo

classDiagram
    class DTDSizeInfo {
        int dtdIndex
        int width
        int height
        bool isBaseBlock
        DTDSizeInfo()
    }

    class EDIDParser {
        // key fields
        int m_Width
        int m_Height
        QStringList m_ListEdid
        QMap~QString,QString~ m_MapCh
        QVector~DTDSizeInfo~ m_DTDSizeInfoList
        bool m_LittleEndianMode

        // relevant methods
        void parseScreenSize()
        void parseMonitorName()
        void parseDTDs()
        void parseOneDTD(int startByte, int dtdIndex, bool isBaseBlock)
        QString binToDec(QString strBin)
        QString hexToDec(QString strHex)
        QString getBytes(int line, int byteInLine)
    }

    EDIDParser --> "*" DTDSizeInfo : aggregates
Loading

Flow diagram for updated parseScreenSize logic

flowchart TD
    A[Start parseScreenSize] --> B[Call parseDTDs to fill m_DTDSizeInfoList]
    B --> C[Read width15 from EDID byte 15H and convert to mm]
    C --> D[Read height16 from EDID byte 16H and convert to mm]
    D --> E[Initialize rightWidth = -1 and rightHeight = -1]
    E --> F{Iterate each DTDSizeInfo in m_DTDSizeInfoList}
    F -->|More DTDSizeInfo| G{Is width and height within 10mm of width15 and height16}
    G -->|Yes| H[Set rightWidth and rightHeight from this DTDSizeInfo and break loop]
    G -->|No| F
    F -->|No more DTDSizeInfo| I{rightWidth == -1 and rightHeight == -1}
    H --> I
    I -->|True| J[Set m_Width = width15 and m_Height = height16]
    I -->|False| K[Set m_Width = rightWidth and m_Height = rightHeight]
    J --> L[Apply specialComType adjustment to m_Width and m_Height if needed]
    K --> L
    L --> M[Compute inch from m_Width and m_Height]
    M --> N[Format m_ScreenSize string]
    N --> O[End parseScreenSize]
Loading

File-Level Changes

Change Details Files
Rework screen size calculation to use all DTDs and reconcile with base 15H/16H size before final width/height selection.
  • Invoke a new parseDTDs() helper at the start of parseScreenSize() to populate a list of DTD-derived physical sizes.
  • Compute base EDID sizes from bytes 15H and 16H (in cm converted to mm) and search the DTD list for a size within 10mm of the base size.
  • Select the first matching DTD size as the authoritative width/height, or fall back to the base 15H/16H size when no close match is found, replacing the previous single-DTD byte66–68 parsing and simple delta check logic.
  • Maintain existing inch computation and screen size formatting, with only indentation changes to the QString::arg chain.
deepin-devicemanager/src/Tool/EDIDParser.cpp
Introduce reusable parsing for DTD physical size and store results in a new DTDSizeInfo list.
  • Add a DTDSizeInfo struct to capture DTD index, width, height, and whether it comes from the base block, and store a QVector of these in EDIDParser.
  • Implement parseDTDs() to clear the DTD size list and iterate over up to four DTDs in the base EDID block, calling parseOneDTD() for each starting byte.
  • Implement parseOneDTD() to validate a DTD via its pixel clock, compute the correct byte offsets depending on endianness, extract bytes 12–14, decode physical width/height in mm from those bytes, and append a DTDSizeInfo entry to the list.
deepin-devicemanager/src/Tool/EDIDParser.cpp
deepin-devicemanager/src/Tool/EDIDParser.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Consider keeping parseDTDs and parseOneDTD private (and potentially nesting DTDSizeInfo inside EDIDParser) to avoid exposing internal parsing details that are only used as implementation helpers.
  • In parseScreenSize, you only fall back to the base size when both rightWidth and rightHeight remain -1; if one dimension matches the base size tolerance and the other does not, the mismatch is silently ignored—clarify whether this is intentional or adjust the logic to handle partial matches more explicitly.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider keeping `parseDTDs` and `parseOneDTD` private (and potentially nesting `DTDSizeInfo` inside `EDIDParser`) to avoid exposing internal parsing details that are only used as implementation helpers.
- In `parseScreenSize`, you only fall back to the base size when *both* `rightWidth` and `rightHeight` remain `-1`; if one dimension matches the base size tolerance and the other does not, the mismatch is silently ignored—clarify whether this is intentional or adjust the logic to handle partial matches more explicitly.

## Individual Comments

### Comment 1
<location path="deepin-devicemanager/src/Tool/EDIDParser.cpp" line_range="207-214" />
<code_context>
-        m_Width = ((byte68_val & 0xF0) << 4) + hexToDec(s66).toInt();
-        // 高度 = (byte68低4位 << 8) + s67
-        m_Height = ((byte68_val & 0x0F) << 8) + hexToDec(s67).toInt();
+    parseDTDs();
+    int rightWidth = -1;
+    int rightHeight = -1;
+    // edid中的  15H和16H是基本屏幕大小(单位cm), 与 Detailed Timing 相差超10mm 则用15H和16H的。
+    int width15 = hexToDec(getBytes(1, m_LittleEndianMode ? 5 : 4)).toInt() * 10;
+    int height16 = hexToDec(getBytes(1, m_LittleEndianMode ? 6 : 7)).toInt() * 10;
+    // 从所有的详细尺寸中寻找接近基本屏幕大小的值
+    for (int i = 0; i < m_DTDSizeInfoList.size(); ++i) {
+        if (abs(m_DTDSizeInfoList.at(i).width - width15) < 10
+            && abs(m_DTDSizeInfoList.at(i).height - height16) < 10) {
</code_context>
<issue_to_address>
**issue:** Guard against empty or zero-valued DTD size entries when deriving the final screen size.

Because `parseScreenSize` now relies entirely on `m_DTDSizeInfoList`, cases where DTDs (or 15H/16H) report 0 for width/height can lead to `m_Width/m_Height` being set to 0, yielding a 0-inch screen. To avoid this, filter out DTD entries with width/height == 0 when populating `m_DTDSizeInfoList` or before the selection loop, and similarly ignore 0-valued 15H/16H when matching or falling back.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +207 to +214
parseDTDs();
int rightWidth = -1;
int rightHeight = -1;
// edid中的 15H和16H是基本屏幕大小(单位cm), 与 Detailed Timing 相差超10mm 则用15H和16H的。
int width15 = hexToDec(getBytes(1, m_LittleEndianMode ? 5 : 4)).toInt() * 10;
int height16 = hexToDec(getBytes(1, m_LittleEndianMode ? 6 : 7)).toInt() * 10;
// 从所有的详细尺寸中寻找接近基本屏幕大小的值
for (int i = 0; i < m_DTDSizeInfoList.size(); ++i) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Guard against empty or zero-valued DTD size entries when deriving the final screen size.

Because parseScreenSize now relies entirely on m_DTDSizeInfoList, cases where DTDs (or 15H/16H) report 0 for width/height can lead to m_Width/m_Height being set to 0, yielding a 0-inch screen. To avoid this, filter out DTD entries with width/height == 0 when populating m_DTDSizeInfoList or before the selection loop, and similarly ignore 0-valued 15H/16H when matching or falling back.

-- Get all screen size and judge it by base screen size.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-354275.html
@GongHeng2017 GongHeng2017 force-pushed the 202603261721-master-fix branch from 53b1b55 to b0fb9cc Compare March 26, 2026 12:30
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@GongHeng2017
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot deepin-bot bot merged commit 348f44d into linuxdeepin:master Mar 26, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants