Skip to content

Fix native panel resolution detection for smooth scaling - #86

Open
dboleslawski wants to merge 1 commit into
didriksg:mainfrom
dboleslawski:fix/edid-native-resolution
Open

Fix native panel resolution detection for smooth scaling#86
dboleslawski wants to merge 1 commit into
didriksg:mainfrom
dboleslawski:fix/edid-native-resolution

Conversation

@dboleslawski

Copy link
Copy Markdown
Contributor

Summary

  • Detect the native panel resolution from EDID and display-registry metadata, with display modes and current pixel dimensions as fallbacks.
  • Use the native dimensions and aspect ratio when generating smooth-scaling modes.
  • Include DisplayPixelDimensions in the macOS display override so smooth scaling can activate after display reinitialization without requiring a reboot.
  • Show the detected native panel resolution directly below the Smooth scaling control.
  • Cache display presets outside SwiftUI view rendering to avoid layout-time preference reads and crashes.
  • Add tests, localization, and documentation for native-resolution detection and overrides.

Testing

  • make check
  • Verified native resolution detection as 3840 × 2160 on a Samsung S32B80P.
  • Verified enabling Smooth scaling and reinitializing the display without rebooting.

@didriksg didriksg left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is the right fix for the right bug. I've seen the 1024x768-as-native case and never traced it back to the mode list being the only source I had. DisplayPixelDimensions is the piece I was missing, and I checked the encoding against Apple's own overrides (DisplayVendorID-610/DisplayProductID-9220 carries 00000A00 00000640 for a 2560x1600 panel), so that part is exactly right.

Four things before I can merge it.

  1. The 6720 cap in smoothScaledLogicalSizes kills mirror mode on Apple Silicon. MirroredModeService.beyondCapStops (from #65, merged a few days ago) draws its stops from that same function and keeps only the widths between the enumerable HiDPI top, around 3360, and native. Capping the source list at 3360 leaves it nothing to keep: for a 5120x1440 panel that's 109 stops down to 0, so the slider loses everything above 3360 and PresetService can no longer restore a preset captured while mirrored. The cap itself is a good idea and I want to keep it. It's a no-op at or below 3360 logical width and a real improvement on 4K, where it trims about 30 modes that could never enumerate. It just needs to be a parameter, because the beyond-cap sizes are rendered on a virtual display where the cap doesn't apply, which is the whole reason that path exists.

  2. On Apple Silicon the early break in detect can never fire. It needs both an EDID hit and a registry hit, but DCP doesn't publish the raw bytes, so edid stays nil and every detection walks the whole tree. On my M4 Max on 26.5.1 that's 2843 entries and 540 ms for a successful read of my AOC, and loadDetails waits on it before starting the mode list, once per display on every connect. Could you match on the display nodes instead (IODisplayConnect, or the DCP AV service), relax the break to "found what this platform can give", and let the mode-list task run in parallel? The DisplayAttributes level you read the NativeFormat pair from is correct, that matched exactly here. Also worth knowing: through a DisplayLink hub there's no EDID node in the registry at all, so the display-modes fallback is load-bearing there rather than vestigial.

  3. The manual override doesn't take effect until the display reconnects. Its real job is feeding nativeAspect into DisplayMode.availableModes, which decides which hidden CGS HiDPI modes get merged in, but setPanelResolutionOverride only clears the view caches. Someone corrects a wrong panel size and the modes they were trying to unlock still aren't there. It needs a loadDetails() or HiDPIService.refreshModes(for:) after the write.

  4. In inspect, edidResolution = EDIDParser.preferredResolution(from: data) assigns even when the parse returns nil, so a second identity-matching entry with an unreadable EDID wipes a good result. Assign only on success.

Small one: the README and docs/native-resolution.md both say "Resolution > Panel resolution", but the row is labelled "Native resolution".

Separate ask: could you pull the DisplayProfileController preset caching into its own PR? It's a real crash fix and unrelated to the title, and I'd rather land it now than have it wait on the rest of this.

Three things I'm not asking you to change, just so you know I looked at them. Taking the largest EDID timing across the base DTD and the CTA native entries inverts the spec's priority, which is clearly deliberate and handles the 1080p-base/4K-native TV, but it does mean a 1080p panel whose HDMI receiver flags a 4K SVD as native reads as 4K, and EDID now outranks the mode list. autoEnableHiDPIIfNeeded inherits that, so a 4K panel on a link that only carries 1080p would clear the 2K+ gate and prompt for admin on connect. And resolvedPanelResolution rebuilds an array over availableModes on every read of nativeResolution, which gets called from view bodies, so it's worth memoizing alongside availableModes at some point.

One style note: VirtualDisplayView.customField already has a panel-styled numeric field with digit filtering. The override editor's .roundedBorder fields and .borderedProminent Save button read like a different app inside the panel.

didriksg pushed a commit that referenced this pull request Sep 1, 2026
DisplayPresetView computed its preset list and the active preset inside body, through MonitorPanel KVC reads. Those reads can pump the main run loop, and a screen-parameter notification delivered from inside that nested loop sets published state mid-update, which trips AttributeGraph's precondition and aborts the app. DisplayProfileController.reload() now takes one snapshot of the presets and the active index from its .task, and the view renders from that cache and updates it after a successful switch.

Split out of #86.
@dboleslawski
dboleslawski force-pushed the fix/edid-native-resolution branch from 924bf23 to df02f3d Compare September 1, 2026 13:56
@dboleslawski

Copy link
Copy Markdown
Contributor Author

Thanks — I’ve addressed all of the requested changes:

  1. Parameterized the 6720 backing cap. Physical override generation keeps the cap, while MirroredModeService requests the uncapped grid with enforcePlatformBackingLimit: false. Tests cover both the capped physical ladder and the 109 beyond-cap stops for a 5120×1440 panel, including preset routing back through mirror mode.

  2. Made detection targeted and concurrent. PanelResolutionDetector now queries only the display-specific service classes (AppleCLCD2, IOMobileFramebufferShim, and IODisplayConnect), returns once the matching node provides the evidence available on that platform, and runs alongside mode enumeration. IODisplayConnect and the mode-list fallback remain in place for DisplayLink/no-EDID cases.

  3. Made manual overrides take effect immediately. Saving or clearing the override now serializes with mirror teardown, reloads DisplayInfo details/modes, clears the controller caches, and refreshes the smooth-scaling state without requiring a physical reconnect.

  4. Preserved valid EDID evidence. EDID data is recorded only when parsing succeeds, so a later identity-matching but unreadable entry cannot erase a good result. There is a regression test for this.

Also addressed the smaller points:

  • README and documentation now consistently say Native resolution.
  • The override editor reuses the panel’s shared digit-filtering PanelNumericField styling and a regular bordered Save button.
  • The unrelated DisplayProfileController caching crash fix was split out and has now merged as Avoid preset reads during SwiftUI layout #87.

I left the three explicitly non-requested observations unchanged: the deliberate largest-native-timing EDID/CTA selection, its autoEnableHiDPIIfNeeded implication on bandwidth-limited links, and memoizing resolvedPanelResolution (best kept as a focused follow-up).

Because applying the override exercises the existing soft-reconnect and mirror teardown paths, I also hardened those paths against display-ID reassignment, finite WindowServer timeouts, late virtual-display registration, and uncertain display enumeration. The cleanup/recovery paths were independently audited after the changes.

Validation is green: SwiftLint, the full test suite, localization-key validation, an Intel (x86_64) compatibility build, and git diff --check.

@didriksg

didriksg commented Sep 1, 2026

Copy link
Copy Markdown
Owner

I got the 5K2K on a direct cable, and it turned up something that changes the shape of this. My Dell U4919DW is a 5120x1440 panel, and Apple's own metadata reports it as 1920x1080. Yesterday I said the win only shows on a panel whose mode list lies. On this one it is the other way round.

image

The node is genuine, not a phantom: IOService:/AppleARMPE/arm-io@10F00000/AppleH16GFamilyIO/dispext0@A0000000/IOMobileFramebufferShim, ProductName "Dell U4919DW", matching LegacyManufacturerID 4268, ProductID 41223 and SerialNumber 808995916, with NativeFormatHorizontalPixels 1920 and NativeFormatVerticalPixels 1080. No raw EDID, as usual on DCP. It also carries HasHDMILegacyEDID, which I suspect is the cause, but whatever the reason the value is wrong.

Since the resolver ranks registry above the mode list, that is what wins. I built the branch and ran it on the panel. Against 1.5.0 on the same display, the PR build:

  • reports "Native resolution: 1920 × 1080" in the new row
  • moves the Default marker from 5120 × 1440 to 1920 × 1080, and the scaling slider reads "1920 × 1080 · 100%"
  • drops 4608×1296, 4096×1152, 3840×1080 and 3360×946 out of the resolution list entirely
  • turns Smooth scaling off with the administrator-password warning back, because the override already on disk for this Dell holds 161 scale-resolutions and the capped ladder is 51

isPlausible cannot catch this, since 1920x1080 is a perfectly reasonable panel size. It also takes mirror mode out a second time, independently of the cap, because the 32:9 ultrawide gate now sees 16:9.

I do not think this sinks the approach. The premise that the largest-mode heuristic can lie is right, I have seen it do exactly that. What this display shows is that the registry can lie too, so a fixed ranking is the wrong shape. It needs corroboration: a candidate native smaller than a mode the panel demonstrably drives at 1:1 should be rejected the same way an implausible aspect is. I checked both variants against this display. Rejecting a candidate below the largest 1:1 mode fixes it, and still accepts a genuine 3840x2160 claim on a panel whose list only offers 1024x768. An aspect-family check also fixes it here, but it would reject that good 4K claim, so the area rule looks like the sturdier of the two.

Separately, point 1 is now confirmed on hardware rather than on paper. With the correct 5120x1440 native and this panel's real HiDPI top of 3360, the beyond-cap stops go from 109 to 0 once the cap is in.

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.

2 participants