Add place and route-stop labels to vector maps - #5853
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb57c0ad79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| addSymbolRule(s, "transportation_name", "name", label, halo).zoomRange(12, 24); | ||
| addSymbolRule(s, "road", "name", label, halo).zoomRange(12, 24); | ||
| addSymbolRule(s, "road_label", "name", label, halo).zoomRange(12, 24); | ||
| addSymbolRule(s, "park", "name", label, halo).zoomRange(12, 24); |
There was a problem hiding this comment.
When the park source layer contains its usual polygon geometry, TileRenderer.anchorOf() anchors the label at the arithmetic mean of the first ring's vertices. That point is not guaranteed to lie inside a concave polygon or outside its holes, so enabling this rule can draw a park name over unrelated map content—or omit it when the computed point falls outside the tile buffer. Use a guaranteed interior point or a proper polygon-label anchor rather than the vertex average.
Useful? React with 👍 / 👎.
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
shai-almog
added a commit
that referenced
this pull request
Sep 17, 2026
"Add place and route-stop labels to vector maps" (#5853) changed TileRenderer, MapStyle, MapView and Routing and reseeded no screenshot baseline on any port, so master has been red on build-macos ever since: NativeMapFallback, RealOsmVector, VectorMapDarkStyle, VectorMapMarkers and VectorMapShapes, pass=183 fail=5 skip=4, identical on master and here. Every frame was reviewed before adopting it. Each keeps its own test content -- the single marker, the three markers, the circle/polyline/ rectangle overlays, the dark palette with light-on-dark label text -- and gains only the street and place labels the feature adds. No regression. Adopted from the runner's own output. The five PNGs are byte identical between master's run (35188870596) and this branch's (35239993396), so the desktop theme work in this branch does not touch them and it does not matter which run they came from; the committed files verify against both. Only macOS noticed. Every other port ships a .tolerance sidecar for these tests (maxChannelDelta=40, maxMismatchPercent=10.0) and scripts/macos has no tolerance files at all, so it compares exactly. The labels cover under 10% of the pixels, which means Android, Linux, iOS, JavaScript and Windows absorbed the change silently and their goldens now depict a map the app no longer draws. That decay is real but it is not failing, and reseeding it needs frames those passing runs do not upload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shai-almog
added a commit
that referenced
this pull request
Sep 18, 2026
* Gate the macOS Aqua theme on master Its golden set is captured, reviewed and committed, so it joins Windows and GNOME on the automatic trigger. It has no BASELINE yet -- the hosted macOS runner queue has not yielded a scoring slot in several hours -- and enabling it anyway is safe rather than a shortcut. FidelityGate treats a pair with no baseline entry as new: it prints the score and passes. So the first run that gets a runner establishes the numbers instead of failing on their absence. Verified rather than assumed, by running the leg locally against the committed goldens with no baseline file present: 60 pairs scored, "[gate] OK", exit 0. When that run lands, record its baseline from its own fidelity-compare.json -- from the runner that scored it, never locally. A Mac-recorded baseline failed the gnome gate on eighteen pairs because the slider comes out a pixel taller on Linux, and a locally recorded baseline passes locally forever, so the mistake is invisible until CI disagrees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Restore the files master's revert removed The merge in the previous commit took master's deletions for every file that did not conflict, which emptied most of the change out of this branch. Restored from a401e5d, the branch tip recorded before the revert, so the branch now carries the complete work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fidelity spec: complete the license headers ComponentSpec, FidelitySpec and FidelitySpecParser carried a header that was missing its "DO NOT ALTER OR REMOVE COPYRIGHT NOTICES" line, so the gate read it as unsupported. Pre-existing, and only in scope because this change touches those files. Replaced verbatim from a file that passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Windows port: the dark-mode native was outside the extern "C" block cn1_windows_window.cpp is C++ and wraps its whole body in an extern "C" block that closes near the end of the file. I appended the native after it, so it got C++ name mangling while the generated C calls the unmangled name. It compiles and the LINKER fails: lld-link: error: undefined symbol: com_codename1_impl_windows_WindowsNative_systemUsesDarkTheme___R_boolean Worth recording that scripts/check-native-signatures.sh passed on this. It verifies the NAME encodes the Java signature correctly, which it did; linkage is a different property and the verifier does not look at it. The only thing that reported this was a real Windows build, which is what the cross-compile leg is for. The Linux equivalent needs no change: cn1_linux_window.c is C, where there is no mangling to avoid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Restore the macOS gate, lost in the branch rebuild Codex caught this: the gated leg set was still gnome+windows. The macOS gate commit was cherry-picked onto this branch and then silently undone, because the merge with master restored these two files from a401e5d -- the branch tip recorded BEFORE the cherry-pick. My own check for it was wrong too: I grepped for the string and matched the `all)` case one line below, so the file read as correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * COVERAGE.md: remove a section duplicated by the merge The Android and Desktop sections appeared twice, the second copy carrying the stale numbers from before the widget fixes (80.8/84.7, macOS "not captured yet"). A merge artifact from rebuilding the branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Act on five codex findings, all of which held up Each was checked against the code before being acted on; all five were real. **The dark-mode native could not link** (P1). Already fixed in 9cc51f2 -- cn1_windows_window.cpp is C++ and the definition sat after the extern "C" block closed. **Bundling MacOSAquaTheme.res did not make the Mac port use it** (P1). MacImplementation inherits IOSImplementation.installNativeTheme(), whose only choices are iOSModernTheme, iOS7Theme and iPhoneTheme, so a native Mac application installed an iPhone design language however its hints were set. IOSImplementation gains a protected nativeThemeResourceName(mode) hook that returns null -- iOS behaviour unchanged -- and MacImplementation overrides it to answer MacOSAquaTheme for auto/aqua/native while letting modern/ios7 fall through to the iOS chain, so a project that deliberately wants the iOS look keeps it. **Hover was never cleared when the pointer left the window** (P2). Motion simply stops, and Form only re-pointed its tracked hover when a DIFFERENT component was reported, so the last control stayed lit with the cursor elsewhere. Windows now arms TrackMouseEvent and handles WM_MOUSELEAVE; Linux connects leave-notify-event. Two things that had to be found by reading rather than by taking the review at face value: the GTK handler would have been dead code without adding GDK_LEAVE_NOTIFY_MASK to the widget's event mask, and Form.pointerHover only updated hover inside `if (cmp != null)`, so the leave event would have done nothing at all. The update now happens unconditionally, and the tooltip lookup just above it is guarded -- a null cmp is reachable for the first time. **getFlattenedStyle had no hover branch** (P2). A UIID deriving from one that declares .hover, without redeclaring it, merged the parent chain with its own raw (empty) hover map and silently lost the parent's rollover colours. Hover now dispatches like every other state. **The Windows leg could not find its JDK** (P1). setup-java hands back a native C:\... path and Git Bash's `test -x` does not resolve it, so the leg would exit 25 without rendering a tile -- a "no java" message on a runner that has one. Normalized with cygpath, the way the fidelity app's own mvnw already does, with the .exe suffix handled rather than assumed. **The documented desktop default was wrong** (P2). The guide said auto/native; resolveDesktopNativeTheme returns legacy when the hint is unset. Codex offered either fix and the doc is the correct one to change: keeping an existing desktop project on the theme it has always loaded is the deliberate back-compat policy, not an oversight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Windows capture app: fix the float compare, push back on the other nine github-code-quality raised ten findings in this file. One holds up and is fixed; nine do not, and the reasons are written into the file header rather than left in a review thread nobody reads. FIXED. RasterizationScale was compared with != 1.0. It is a double, and a display that is 1x in every way that matters can report a value a hair off it, which would raise a blocker about a scale nobody set. Now a tolerance. The `!= 0` beside it stays exact on purpose: that is not a measurement, it is the sentinel for "XamlRoot was null", assigned literally and exactly representable. PUSHED BACK, "calls to unmanaged code". There is no managed equivalent for any of it. The program exists to photograph what the compositor actually put on screen -- PrintWindow with PW_RENDERFULLCONTENT, BitBlt with CAPTUREBLT, DwmFlush, TrackMouseEvent. A managed screenshot API answers a different question, "what does XAML think it drew" rather than "what did DWM composite", and that exact substitution is what made an earlier version of this app report success while capturing unstyled controls. PUSHED BACK, "generic catch clause", six times. Narrowing them would make the app worse at its one job. It is a capture probe whose contract is that it refuses to produce a reference set it cannot vouch for, so every failure has to become a recorded blocker rather than an escaping exception that kills the process with no manifest. Each broad catch is paired with a documented fallback or with a blocker naming the stage and the HRESULT. A typed list would have to enumerate every COM failure a hosted runner can produce, and the ones it missed would crash silently instead of being reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Developer guide: contraction the prose gate asked for Vale: "Use 'doesn't' instead of 'does not'" in the desktop theme default I just rewrote. The same rule PR #5841 applied to the earlier text in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Scope the progress-bar value to desktop rows; mobile was regressed Setting the CN1 progress bar to 60 to match the desktop reference apps changed it for the MOBILE rows too, and their goldens are captured at 0.5 -- RefWidgets.java sets setProgress(50) and NativeRef.swift sets p.progress = 0.5. So a desktop fix quietly moved the iOS and Android suites off their own references. Now scoped by row: slider 0.5 everywhere, progress 0.6 on desktop and 0.5 on mobile, each mirroring the reference app that captured that set. Desktop scores are unchanged by the scoping (Windows mean 82.2%, progress 74.5/74.7). Found by asking whether an unrelated Android leg's failure could be mine rather than assuming the emulator flaked. The emulator trouble in that run was real and separate; this was underneath it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Act on six more review findings; four were real bugs **Aqua was unreachable** (P1, codex). The hook added last round was dead code: MacOSBuildHints.THEME_MODES excluded "aqua" and "native", so the documented macos.themeMode=aqua was sanitized to "modern", and the default was "modern" too. Whitelist extended, default changed to aqua, and the whitelist's own fallback follows it -- a typo used to produce an iOS look on a Mac silently. Scoped deliberately: an EXPLICIT nativeTheme=modern is still honoured as written, so only the unset case moves. The two tests that pinned "modern" are updated rather than deleted; their original reason (modern was the only theme declaring @darkModeBool) no longer holds, because MacOSAquaTheme declares it and ships 326 $Dark entries. **Linux dark mode always answered light** (P1, codex). gtk-application-prefer-dark-theme is what the APPLICATION asks for, not what the desktop reports, and it stays false unless the app sets it -- so every $Dark entry in Adwaita was unreachable on a GNOME desktop in dark mode. Now reads org.gnome.desktop.interface color-scheme, which is what the user's toggle writes. Two aborts avoided on the way, neither raised by the review: g_settings_new kills the process when the schema is absent (a minimal container, a non-GNOME desktop) and g_settings_get_string does the same for a missing key (color-scheme arrived in GNOME 42), so both are looked up first. **Hover never reached secondary windows** (P2, codex). The drain loop dropped every hover with windowId > 0 even though windowPointerHover already routes by id, and the secondary Win32 and GTK handlers discarded buttonless motion in the first place. Both now emit hover and a leave, and the Java side routes rather than filters. **JavaSE never cleared hover on exit** (P2, codex). mouseExited hid the tooltip and stopped there, so the last control stayed lit once the pointer left the canvas. It now sends the same (-1,-1) the native ports send. **Pushed back on two.** github-code-quality wants a tolerance on `rasterScale != 0` as well; that one is a SENTINEL assigned literally by `?? 0`, not a measurement, so an exact compare is correct and an epsilon would misread a real 0.0005 scale as "unknown". And another "calls to unmanaged code" on a file whose entire purpose is Win32 interop. Both are answered in the comment block at the top of that file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Don't flip any port's default theme in this change Scope correction, made after CI showed what the flip actually costs: the Windows port screenshot gate failed with 154 of 166 baselines changed, because the pom now staged WindowsFluentTheme instead of the Material placeholder and the port's entire UI restyled. That reseed is legitimate work and it is not this change's work. Flipping a port's theme restyles every screen; it deserves a review of its own rather than riding along inside the change that introduces the theme, and it wants doing once the themes reach their fidelity targets rather than now and again later. The plan this work follows sequenced it last for exactly that reason, and I had quietly brought it forward. So: Windows and Linux stage the Material placeholder again, and the macOS default goes back to modern. What the review asked for is kept -- aqua and native stay in the MacOSBuildHints whitelist, so macos.themeMode=aqua now genuinely selects the Aqua theme through the nativeThemeResourceName hook, which is what codex's finding was actually about. Selectable, just not default. The two tests that pin the macOS default are back to asserting modern; the guide and COVERAGE.md say plainly that no port defaults to its native theme yet, and each flip is one line when someone wants it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Three more codex findings; all three held up **Lead components could never show hover** (P2). Component.getStyle()'s hasLead branch returns unSelectedStyle before reaching the hover check, and Form.pointerHover marks the LEAD parent hovered rather than the child under the pointer -- so a MultiButton, SpanButton or toolbar command container could not render a declared hover style at all. Handled inside that branch, in the same position it holds on the main path: after pressed, ahead of focus. **Touch was being reported as hover on Windows** (P2). Windows promotes touch and pen input to tagged mouse messages, so a finger dragging across the window arrived as buttonless motion and lit a control in a desktop-only hover style that nothing cleared until the next pointer event. Dropped at the source in both the main and secondary window handlers, because the tag rides in the event's key field and the hover routing does not read it. Linux needed no change: cn1OnMotion already returns early for a touch source, which is worth knowing rather than assuming. **An ordinary box-shadow would have rendered as no shadow** (P2). Making box-shadow a native primitive routed `box-shadow: 0 2px 4px rgba(...)` -- the ordinary shape of the property, with CSS's default zero spread -- to RoundRectBorder, whose software shadow loop is `for (iter = shadowSpreadL - 1; iter >= 0; iter--)` and runs zero times at zero spread. On any renderer without shape-shadow support the shadow simply vanished. The native path now requires a usable spread, so anything that would render wrong keeps rasterizing: worse looking and correct beats native looking and absent. That last one is latent rather than live -- no theme in the tree declares box-shadow, both mentions are comments -- so it is gated now for the first theme that does, and the compiled .res files are unchanged by it. core-unittests: 6958 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert a review "fix" whose premise was false, and reseed one screenshot I broke the build. The spread>0 guard added to boxShadowIsNativeRoundRect failed CSSBoxShadowNativeBorderTest -- the test that exists to pin exactly the case the guard excluded -- and took build, build-linux-jdk8 and all three build-test legs down with it. I ran core-unittests and the plugin tests after that change and not the css-compiler ones, which are the tests for the file I edited. The finding it came from was wrong on both halves, and I should have checked the premise rather than only the mechanism: - "createRoundRectBorder() leaves shadowSpread at zero, so the software shadow loop runs zero times." RoundRectBorder's constructor seeds shadowSpread to convertToPixels(0.2f). It is never zero unless the CSS sets it, so the loop runs. - "It stores the blur as a negative value via shadowBlur(-calculateShadowRatio(...))." The minus cancels one already inside that method's px branch. The stored blur is positive. Both are now recorded in the code at that site so the guard does not come back. Separately, one screenshot baseline moves: DesktopMode on the Windows port, 165 of 166 matched. It renders a desktop scrollbar instead of the thin mobile indicator, because isDesktop() now answers true -- which is the whole point of the screenshot named DesktopMode. Checked by eye before reseeding, not accepted on the count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Developer guide: drop an adverb the prose gate rejects Vale Microsoft.Adverbs on "deliberately" in the macos.themeMode row. The guide gate treats warnings as build breaking, so this is the whole of the remaining build failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Desktop fidelity: score each tile against its own backdrop, and six hover fixes Six findings from the latest review round, each verified against the code before being acted on. Fidelity measurement (P1). ProcessScreenshots masked BOTH the native golden and the CN1 render with the CN1 tile background. They are two different machines' backdrops and coincide only by luck: Fluent and Adwaita happen to match, but the macOS Aqua goldens are captured on #E7E7E7/#262626 while the theme paints AppKit's nominal windowBackgroundColor #ECECEC/#323232 -- 12 levels apart in dark mode, past CONTENT_TAU, so every native backdrop pixel was classified as widget and the whole tile dominated every Aqua dark score. The native backdrop now comes from the golden set's own capture-manifest.json. Mobile sets ship no manifest and fall back to the CN1 value, so iOS and Android scores are unchanged by construction. Hover state. A $Dark-only hover declaration no longer reports as declared in light mode -- the lookup that followed asked for the nonexistent light key and got a style built from blank defaults, so hovering dropped the control to the default colours. The interactive-scrollbar hover is now cleared on window leave alongside the component hover; it was updated only when a component was found, so a thumb stayed lit after the pointer left. JavaSE sends its leave through windowPointerHover so a secondary window's control does not stay hovered. Fidelity renderer. The iOS glass alpha is keyed on the spec's own row id rather than the mapped kind, so DesktopAccentButton is not made translucent against opaque native accent fills. The hover style's margin is normalised to the normal style's, because the branches zero their margin through getAllStyles(), which by design excludes hover -- the hover tile was laid out at a different offset and size from the control it is measured against. The dark-only hover test asserted the bug; it is split into a light half that requires fall-through and a dark half that requires the declared colour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Desktop fidelity: stop a missing baseline from reading as a passing gate The macOS leg is listed as gated and has committed goldens, but no baseline. FidelityGate reads a missing baseline file as an empty map, so every pair is "new", its score is printed and the run exits 0 -- measured here: two pairs scored, exit 0, nothing ratcheted. The leg still fails on a missing golden, an undelivered render, a blank pair or a size mismatch, so it is not toothless, but a fidelity DROP is exactly what it cannot catch, and nothing said so. An invented baseline is not the fix: a Mac-recorded one failed the gnome gate on eighteen pairs because the slider comes out a pixel taller on Linux, which is why these have to come from the runner that scored them. So make the half-armed state loud instead. cn1ss_process_fidelity now detects the absent file, reports it in the log and the job summary, and writes the baseline the run WOULD record into the job artifact through FidelityGate's own writer, so seeding the set is downloading that file and committing it. The writer refuses a partial or broken run, so a seed only appears for a run that scored cleanly, and the real gate still runs afterwards unchanged. Sets that already have a baseline take none of this path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Compile the themes the desktop gate scores, and stage the ones the sync job writes Three problems, all of them "the build reported success without doing the work". The developer guide's hover section includes a tagged CSS block that master deleted while this branch was reverted off it -- "Drop the hover CSS block the revert orphaned" in #5844, correctly, since nothing referenced it there. The merge keeps master's deletion and this branch's include, so the snippet gate fails on a dangling tag. The block comes back with the section that uses it. Reproduced locally by merging master first: 1134 blocks, the same count CI reported, failing on the same line. The desktop fidelity workflow never regenerated the themes. theme.css is the source of truth and Themes/*.res is committed compiler output, and run-desktop-fidelity-tests.sh puts Themes/ first on the classpath on purpose so no stale copy elsewhere can win -- which also means a push that changes only native-themes/<platform>/theme.css was scored against the PREVIOUS compiled theme and could pass without the change ever being rendered. The Android and iOS legs already carry this step, with a comment naming the same failure. native-themes-sync.yml staged a hand-written list of two paths. The script now writes seven tracked files -- five themes and two committed port mirrors -- so a desktop-only CSS change regenerated three .res files, staged none of them and reported "nothing to commit", which is also what a genuine no-op says. The script now records what it wrote and the job stages that, so the list cannot go stale again; untracked outputs are filtered out because the same script mirrors into the JavaScript port's gitignored asset directory. Measured on this tree: 12 outputs recorded, 7 tracked and staged, 5 skipped. An empty result is now an error rather than a quiet success. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Developer guide: take sql-entry and storage-content from the runner These two figures arrive from master, not from this branch. #5847 added them and was merged at 09:08 while its own CI was failing on exactly these two files, so every branch that picks master up now fails "Verify generated developer guide figures are current" on them. Not this branch's doing, and checked rather than assumed: the figures this branch's run produced are BYTE-IDENTICAL to the ones dg-figures-3 produced, and that branch carries none of this work. Locally, against the merged tree, 80 of the 82 figures match the runner artifact byte for byte and only these two differ. The difference is position, not content: same 409x919 size, same dialog, but it sits about fourteen pixels lower in the runner's render and the committed copy carries a black band along the bottom edge -- 10.9% and 7.2% of pixels at a full 255 channel delta, which is a moved edge rather than antialiasing. That is the signature compare-screenshots.py documents for an image committed from a local render, and the remedy it names is the runner's output, which is what these are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Put the hover flag where getStyle reads it, and give Window the same tracking Four follow-ups from review, each verified against the code and then against a failing test before being fixed. Hover was recorded on the LEAD PARENT, which is what the pointer resolves to, while Component.getStyle() consults the lead COMPONENT: a component inside a lead hierarchy returns out of its lead branch after asking lead.isHovered() and never reaches the plain hover check. So a MultiButton, a SpanButton or a toolbar command container could not show a hover style its theme declared. The flag now goes on the lead component, which is the rule the pressed state already follows -- LeadUtil.pointerPressed delivers to leadComponentImpl and getStyle() asks lead.isPressedStyle(). leadComponentImpl answers the component itself when there is no lead, so an ordinary component is unaffected. Window is not a Form -- it extends Container -- and its pointerHover only forwarded the event, so routing the native per-window hover to it was not enough on its own: nothing there recorded what the pointer was over. Rather than a second copy of the logic, the tracking moves into HoverTracker, which Form and Window both own an instance of. That also gives a window the leave handling and the scrollbar-thumb clearing Form has, and clears the state on dispose the way Form does on deinitialize. Both are covered by HoverDeliveryTest, and the tests were checked to FAIL without the fix: with the flag put back on the lead parent the row stays at its normal colour, and with the window's tracking removed it records nothing. The fidelity renderer no longer normalises the hover margin. DesktopTileRunner zeroes margins AFTER build() returns, so the copy was overwritten a line later for every component whose branch does not zero its own -- DesktopSwitch and DesktopSlider keep the theme's 0.5/0.8mm. The runner now zeroes the hover style alongside the rest, which is one place for every component instead of per branch. SSIM still saw both raw backdrops. Masking fillSim was not enough because the headline multiplies the two terms, and the backdrop is most of a tile: with the Aqua dark pair the whole-tile SSIM carried a constant penalty for a difference that is not the widget. The native tile's backdrop is now repainted in the CN1 backdrop colour before SSIM, using the same CONTENT_TAU the content mask uses so no widget pixel is touched. Measured on a synthetic pair with an IDENTICAL widget: 96.05% before, 100% after, and a set whose backdrops already agree takes none of this path and scores exactly as it did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Load the Windows override layer that exists, and wait on a latch Two more from review. getPlatformOverrides ended with "windows" while getPlatformName reports "win". Resources.openLayered appends the value literally as <resource>_<name>.ovr and the resource editor writes the Windows override as _win.ovr, so the entry asked for a file nothing produces and the port-specific layer this method exists to load could never be found. The macOS and Linux ports already pair the two the same way -- mac/mac and linux/linux. Verified in the compiled class: the array now holds "win" from the same constant pool entry getPlatformName() returns. DesktopTileRunner polled two ordinary arrays that the EDT writes. A plain field written on one thread has no happens-before edge to a read on another, so nothing required the waiting thread to observe the render finishing: a JVM may keep serving the initial values, wait out the whole 120 seconds and report "no tiles were rendered" for a run that rendered everything, or swallow the real exception behind that message. It waits on a CountDownLatch now, which supplies the edge, and the deadline still bounds a genuine hang. This is a host-side test runner with a real cross-thread handoff, not framework code -- the Codename One rule that the EDT is single threaded and needs no synchronization is about the framework, and does not reach a JVM main thread waiting on the EDT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Linux: reseed the DesktopMode baseline now that the port is a desktop The Linux port now answers true from isDesktop(), so the scrolling container in this screen paints a desktop scrollbar -- a track and a thumb down the right edge, with the content narrowed to make room -- instead of the mobile scroll indicator the baseline was captured with. The same change required the same reseed on the Windows port, which is already in this branch; the Linux pair was missed. Both arches, from the runners that rendered them: x64 into scripts/linux/screenshots and arm64 into scripts/linux/screenshots-arm, which is the mapping the workflow documents. The two renders are byte-identical to each other, which is the expected answer for a pure layout change. Nothing else moved. Against the same artifacts every other committed baseline is either byte-identical or inside the default tolerance the gate already applies (channelDelta 4, mismatch 0.30%): four Tabs/graphics screens differ raw and the gate reports them as matching, so they are deliberately left alone rather than churned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Link advapi32 unconditionally, and catch hover up when a gesture ends The Windows CMake put advapi32 only inside the optional WebView2 block, while cn1_windows_window.cpp calls RegGetValueW unconditionally to read AppsUseLightTheme for dark mode. An application built without WEBVIEW2_SDK_DIR -- which is every ordinary one -- therefore failed at the LINK step on a symbol our own port references. Our CI never saw it because the cross-compile leg sets WEBVIEW2_SDK_DIR and got the library by accident, which is exactly how the iOS port once lost UniformTypeIdentifiers. Hover is not tracked during a drag: pointerHover returns early while a component is being dragged. If the pointer then stops, no further motion event arrives -- Windows sends none for a stationary cursor -- so the component the drag started on stayed hover-styled and the one under the pointer never lit up. Both Form and Window now resolve the pointer's component at release and hand it to the tracker; resolved rather than dispatched, so a release raises no tooltip and fires no hover callback of its own. Placement was the whole difficulty, and the test is what exposed it. Form's pointerReleased returns from six places inside a try, so the first version -- after the block -- was reached by none of them and the test failed with the old behaviour intact; it now sits in the finally beside the other end-of-gesture bookkeeping. Window has the same shape with two early exits, so the catch-up is a small method called from each of its three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Distinguish touch and pen hover and clear window tooltips * Preserve local hover styles and reject window-leave hit targets * Carry desktop theme choices and platform fonts into packaged JavaSE apps * Handle custom desktop themes and nonpositive shadow spread * Correct native shadow bounds and seed macOS fidelity baseline * Keep desktop font changes opt-in and normalize CSS shadow offsets * Fix inherited hover and preserve progress text sizing * Preserve fractional native shadow geometry without display density * Capture rasterized hover states and parse shadow padding by property * Use native desktop font aliases and reset Linux hover metadata * Match desktop fidelity captures to the native logical pixel scale * Run Fluent fidelity with the Windows 11 reference font inventory * Match desktop control text to native point and logical pixel sizes * Share installed desktop font family across system and native aliases * Complete hover lifecycle and runtime inheritance with native-scale baselines * Keep hover state consistent through callbacks and reusable window hide * Validate fidelity spec and native reference consistency in CI * Bound native shadow offsets and validate hover and fidelity mappings * Keep mobile theme hints independent of desktop opt-in * Express hover inheritance convergence as a bounded while loop * Keep CSS blur halos on the raster shadow path * Release hover targets and tooltips during component teardown * Cancel window-owned tooltips when input is cancelled * Clear Windows hover pressure while preserving pointer source * Preserve Linux pen source across hover and contact events * Release hover animation registrations without canceling application animations * Validate fidelity defaults and reject malformed spec fields * Honor custom theme mode in the desktop simulator skin * Keep scrollbar fades active alongside hover background animations * Protect terminal hover leave events in desktop input queues * Preserve inline-all overrides and theme inheritance in hover styles * Honor programmatically installed custom hover styles * Resolve native macOS font aliases with AppKit weights * Clear hover ownership when an attached subtree is hidden * Refresh progress mode sizing and attach fidelity focus state * Trigger desktop fidelity when theme generators change * Validate fidelity materials and score shared spec changes * Validate fidelity tile dimensions and backdrop declarations * Validate animation frame values and capture filenames * Preserve text input focus and enforce fidelity coverage * Track effective hover animation through lead and style changes * Preserve geometry coverage and mask grouped fidelity backdrops * Retain eraser input and require geometry when seeding baselines * Scope native font configuration to JUnit theme installation * Run desktop fidelity for core and JavaSE source changes * Reconcile hover lead changes and enforce complete desktop captures * Clear collapsed hover state and validate fidelity renderer IDs * Reject backdrop-only glass and lens fidelity captures * Preserve custom progress assets with native themes * Keep progress sizing stable across custom component states * Preserve hover state wrappers when applying CSS derivation * Rebuild dark desktop progress capsule borders with their colors * Preserve legacy macOS font mapping outside Aqua mode * Record Window slider captures from run 35169743013 * Reject Linux host aliases in GNOME fidelity filters * Run Android editor stress teardown on the native UI thread * Add required copyright header to Android editor fixture * Bound every simctl call in the iOS fidelity boot step The Boot simulator step twice hung with no output at all and burned the job's entire 90-minute budget -- on this branch (run 35175449094) and independently on master (run 35130437729), at the identical step. Both ended as CANCELLED, which reads as "a human stopped it" rather than as a failure, and both step logs were empty, so there was not even evidence of which of the four simctl calls hung. Move the step into a script that puts each call on a deadline, samples the hung process before killing it, and recovers once by restarting CoreSimulatorService. The one retry is allowed here because the thing retried is not ours and carries no signal about the code under test; the script's header records that reasoning, the two runs above, and the two simctl call sites in the tree that remain unguarded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add a native theme mode that covers every OS, desktop included Three gaps in the desktop theme hint, all of them things a developer reading DesktopBuild could not find out: - desktop.themeMode accepted "modern" at runtime and the annotation's valuePattern rejected it, so the compiler refused a value the port honours. - There was no single hint for "look like the platform, everywhere". nativeTheme=modern reaches iOS and Android only, and deliberately so -- it predates the desktop themes by years, so a desktop application that set it for its phone builds never asked for its desktop screens to move. ThemeMode.NATIVE is that value: modern plus the desktop. - Nothing said which hint governs which platform, or what each value installs. Both tables now live on the DesktopBuild annotation type, not on the attribute: an attribute's prose is harvested verbatim into the developer guide's AsciiDoc hint table, where a markdown table collapses onto one line with every pipe escaped. "native" had to be taught to every consumer, because an unrecognised theme mode is never an error -- it falls back, silently, and each fallback was wrong in a different direction. IPhoneBuilder mapped it to "auto", which installNativeTheme() resolves to the FLAT iOS 7 theme; the Android runtime fell through to Holo Light. macOS already answered Aqua for it and needed nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop the playground editor tripwire matching a country and a font The smoke job greps CodenameOne/src for the removed browser editor's name. That name is also a principality and a monospace font, and core is four thousand files of framework source, so the bare word over it was never a tripwire -- it was a guarantee of an eventual false positive. It fired: PhoneNumberField's dialing-code table lists the principality, and the job went red on a pull request that had touched neither the editor nor that file. The workflow only triggers on scripts/cn1playground changes, so the table sat there unnoticed for two and a half weeks and then detonated on the first unrelated change that happened to touch the playground tree. Over core the name now counts only where it is shaped like a dependency -- adjacent to '-', '.' or '/'. That is every way the editor is really referenced (package name, JS namespace, loader path) and none of the ways the word occurs in prose or a font stack; checked against five realistic reintroductions and three legitimate uses. The playground's own tree is small and controlled, so the bare name stays forbidden there. No \b in the pattern, deliberately: `git grep -E` honours neither \b nor \< on macOS, so a word-boundary version would match NOTHING on a developer machine and pass for that reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Keep the new theme hint prose inside the developer guide's style gate A build hint attribute's javadoc is harvested verbatim into the guide's hint table, so it is published prose and Vale lints it as such. The two paragraphs added for the native theme mode tripped four alerts -- two Microsoft.Contractions errors on "does not" and two Microsoft.Adverbs warnings on "deliberately" -- and the docs job treats warnings as build-breaking. The DesktopBuild paragraph also carried a parenthetical explaining to the next editor why javadoc link syntax is avoided there. That is an internal authoring note, and it was being published in a customer-facing document. It moves to a // comment ABOVE the /// run, which is invisible to the harvester -- below would be worse than invisible, because a non-/// line discards the pending comment and the attribute would reach the guide with no description at all. Verified with the CI configuration: vale --config docs/developer-guide/.vale.ini --minAlertLevel=suggestion now reports 0 alerts over the whole guide. The two "table cell has been truncated" PDF errors in the same log are pre-existing -- the same count appears in the last run of this workflow that passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Give the gradle distribution download a retry window worth having An Android job died on three HTTP 500s from GitHub releases inside six seconds, and the same URL served a range request perfectly a few minutes later. Three attempts two seconds apart all land inside the same outage window: they cost the runner six seconds, buy nothing, and throw away a sixteen-minute job over a transient upstream error. Four attempts now, with the wait GROWING -- 10s, 40s, 160s. That is the lesson scripts/ci/retry.sh already records for Maven Central 403s, applied to the other download an Android build cannot proceed without. The worst case adds about three and a half minutes before giving up, which is cheap next to the job it saves. The wait is also logged, so a failure says how long it actually tried. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Reseed the macOS map goldens for the vector map labels "Add place and route-stop labels to vector maps" (#5853) changed TileRenderer, MapStyle, MapView and Routing and reseeded no screenshot baseline on any port, so master has been red on build-macos ever since: NativeMapFallback, RealOsmVector, VectorMapDarkStyle, VectorMapMarkers and VectorMapShapes, pass=183 fail=5 skip=4, identical on master and here. Every frame was reviewed before adopting it. Each keeps its own test content -- the single marker, the three markers, the circle/polyline/ rectangle overlays, the dark palette with light-on-dark label text -- and gains only the street and place labels the feature adds. No regression. Adopted from the runner's own output. The five PNGs are byte identical between master's run (35188870596) and this branch's (35239993396), so the desktop theme work in this branch does not touch them and it does not matter which run they came from; the committed files verify against both. Only macOS noticed. Every other port ships a .tolerance sidecar for these tests (maxChannelDelta=40, maxMismatchPercent=10.0) and scripts/macos has no tolerance files at all, so it compares exactly. The labels cover under 10% of the pixels, which means Android, Linux, iOS, JavaScript and Windows absorbed the change silently and their goldens now depict a map the app no longer draws. That decay is real but it is not failing, and reseeding it needs frames those passing runs do not upload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say what the backend servers did when a backend test fails The four server processes each redirect their combined output to a file and nothing ever read one back, so an intermittent failure arrived as "expected: <200> but was: <-1>" and nothing else. That does not separate the three answers that matter -- the server refused the request, the server never saw it, or the server is gone -- and without the distinction there is nothing to debug from. Two vm-tests runs were lost to exactly that. aLargeUploadIsReadWhole answered an empty reply in 0.020s and aZeroMinimumBodyRateDoesNotKillTheConnection in 0.304s, both on a body that needs a second read, and neither says whether a connection was dropped or a process had died. A full local reproduction -- 18 clean runs of the class, plus pipelined, split-boundary, 2MB-upload and 48-connection stress against a locally built server -- did not reproduce either, so the next occurrence has to carry its own evidence. A TestWatcher rather than a message on each assertion, because the next occurrence will not be in a test anyone thought to instrument. Proven to fire by injecting a failure: it prints each server's liveness and the tail of its log, which is how the pool-mode worker counts and the small-upload server's clamped settings became visible at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin the backend TLS test to the certificate it generated openTls() installed a TrustManager whose check methods are empty, which verifies nothing at all -- including that the server presented the certificate it was configured with, the one thing a TLS test is in a position to assert. CodeQL rates it a high-severity alert, and it is the shape every "disable certificate checking" answer on the internet has, so leaving a copy in this repository invites it to be pasted somewhere it is not a throwaway localhost socket. Pinned to the self-signed certificate startTlsServer already writes, as a trust anchor. Path validation only: these sockets reach 127.0.0.1 while the certificate names localhost, and a raw SSLSocket does no hostname check unless one is asked for. Verified both ways, because a trust manager that accepts everything and one that is correct both look like a passing suite: 71/71 with 0 skipped against the real certificate, and pinning a DIFFERENT self-signed certificate turns the four TLS tests into "PKIX path validation failed: Path does not chain with any of the trust anchors". Pre-existing, and identical on master -- it surfaced here only because CodeQL's diff-informed analysis re-read the region after the 72-line diagnostics change above it shifted the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vector maps currently omit street and landmark names from the built-in styles, and
Routing.showRoute()draws only the route line. This change adds both basemap names and visible labels for the journey's start, destination, and intermediate stops.MarkerOptions.label()adds persistent vector-map text above route geometry and pins, with viewport clipping, ellipsis, and collision avoidance. Existing info-window titles retain their behavior.Routing.showRoute()adds labeled stop markers by default and frames their requested locations alongside the road geometry. Requests support custom names andsetShowStopLabels(false), and are snapshotted before asynchronous routing.Without custom names, labels use localized Start, Stop N, and Destination defaults. No reverse geocoding is performed. Native providers receive the names as marker titles; persistent labels are rendered by MapView and its native-map fallback.
Validation: Java 8 Maven
verifypassed on the current master base with 105 map/routing tests, the SpotBugs check, and PMD/Checkstyle report generation. Coverage includes zoom thresholds, curved/multipart roads, custom styles, request mutation, failure handling, hidden/offscreen markers, long labels, and coincident endpoints. Also checked label extraction against the bundled San Francisco OSM tile. Device screenshots were not run.