Native desktop windows - #5556
Open
shai-almog wants to merge 89 commits into
Open
Conversation
Introduces com.codename1.ui.TopLevelContainer, the interface implemented by anything that can sit at the root of a component hierarchy. Today that is only Form; a later commit adds Window, the desktop native-window top level. Every member is chosen from a count of the direct getComponentForm().<method>() chains in CodenameOne/src, so the interface is the measured contract core actually depends on rather than a guess. It is dominated by animation registration, focus, and the layered panes. Every method already existed on Form with an identical public signature, so this commit adds no behaviour and needs no Form change beyond the implements clause and the new asContainer() bridge -- a Java interface cannot extend a class, so without it a TopLevelContainer reference could not be passed anywhere a Component is expected. Deliberately excluded: MenuBar and the soft buttons (MenuBar is coupled to Form's tint, back command and actionCommandImpl), dispose()/isDisposed() (package private on Form, and it means "pop back to previousForm" rather than "destroy this window"), and the mobile navigation surface -- transitions, back command, previousForm, tint and orientation listeners. Members already on Component or Container are reachable through asContainer(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds Component.getTopLevelContainer(), the resolution path that replaces getComponentForm() in code which has to keep working inside a desktop Window. getComponentForm() is untouched and keeps its meaning: it returns the enclosing Form, and null for a component hosted in a Window, because a Window is not a Form. The internals that Component, Container and Toolbar need in order to drive a top level -- the internal animation registry, focus, the revalidate queue, the drag and press state -- are declared package private on Container rather than on an interface. Every method of a Java interface is implicitly public, so an interface would have silently widened Form's public API; Container is the nearest common supertype of Form and Window, so the calls still dispatch virtually with no instanceof. The defaults are inert and Form overrides the ones that mean something to it. Also adds com.codename1.impl.WindowManager, the single facade carrying the whole native windowing contract, reached through one new CodenameOneImplementation.getWindowManager() that returns null by default. This follows getHealth()/getBluetooth()/getCarBridge(), and keeps several dozen methods out of an already very large class. The null return is itself the capability query, so no separate supported flag can drift out of step with it. Only operations every windowing system provides are abstract; the rest have inert defaults so a later addition cannot break an existing port. No behaviour change: no port implements a window manager yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migrates the load-bearing call sites in Component and Container off getComponentForm() and onto getTopLevelContainer(), so they keep working when the root of the hierarchy is a Window instead of a Form. The sites were picked by reading, not by pattern. Two groups: Sites that dereferenced the Form with no null check, and so would have thrown rather than degraded: growShrink and its BGPainter animate loop, the material pull to refresh release, the deinitialize path that unhooks the refresh drag listener, chooseScrollXOrY, moveScrollTowards, and the drop handler that animates the hierarchy. Several of these could already NPE today for a component detached mid animation, so they are now guarded as well as migrated. Sites that were guarded and would therefore have gone quiet -- the worse failure, because each one silently removes a whole feature: all pointer dragging, kinetic and smooth scrolling, drag and drop, focus, the animation manager behind every animateLayout, animated backgrounds, the revalidate-on-style-change gate, and revalidateInternal, which is the root of the layout system. Adds four more package private hooks to Container that these sites need -- getFocused, isRevalidateFromRoot and the directional focus finders -- following the pattern established for the rest: inert defaults on Container, overridden by the top level. Left alone deliberately: fireFocusGained and fireFocusLost reach for getMenuBar(), which a Window has no equivalent of, so the existing null guard already yields the right behaviour there. All 4790 core unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groups the four fields that describe "the thing being painted" -- the dirty queue, its swap buffer, the fill count and the Graphics -- into a PaintSurface. The application's main surface becomes one instance of it, and a later commit gives every native window another. paintDirty() keeps its signature and behaviour and now delegates to a surface-parameterized routine, with paintDirtyWindow() entering the same routine for a window. Having one copy matters: that method carries the clip and paintable-bounds handling from issue #5273, and a per-surface copy would be free to drift. The flush-region hint is routed per surface. Its window form is inert by default rather than delegating to the main-surface version, so an immediate mode port that has not opted in cannot clamp a window's clip against the main window's state. repaint(), cancelRepaint() and hasPendingPaints() keep their signatures, so the JavaSE and Android overrides that call super still compile and behave. cancelRepaint now sweeps every surface, since its callers have no window context. No behaviour change: nothing creates a window surface yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extracts the layer lookup and z-index insertion out of Form into TopLevelSupport, so Window can reuse it instead of carrying a second copy. The logic is moved verbatim, including the getChildrenAsList(true) reads: the comment there is load bearing, since iterating the container directly does not find components while an animation is in progress and the method would then add a duplicate layer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Window is the desktop counterpart of Form: a second native operating system window with its own component hierarchy, focus owner, animations, revalidate queue and dirty region. The main surface stays a Form and is untouched. Desktop is the public API parallel to Display. Display keeps answering "how big is the application's main surface", which is the only question a phone has; Desktop answers "what screens exist and what windows are open". It owns the window registry and hands out Monitor snapshots, and every one of its methods degrades safely where there is no windowing system -- an empty window array, a single monitor describing the main display -- so only constructing a Window throws. Monitor carries per-monitor geometry, work area, density and backing scale, and a Window reports the density and scale of the monitor it is currently on rather than the global one, which is what makes a mixed-DPI desktop render correctly. Event routing packs the window id into the high bits of the event type word. Window 0 is the main surface, and for it the packed word is numerically identical to what it always was, so the wire format, drag coalescing and the stack swap are all untouched. The id is an int chosen by the framework and echoed back by the port, so the off-EDT input path needs no map and no lock. Key repeat and long press now return to the top level the press came from. Fixes a latent infinite EDT spin this makes reachable: handleEvent returned without advancing the offset when it had no form to dispatch to, while the caller loops while (offset < end). It was unreachable only because the public entry points all guard on a non-null current form. skipEvent now drains the packet so the rest of the batch -- which may contain main form events -- still dispatches. Fixes two adjacent bugs the same code path forced into the open: a key or pointer release aimed at a different form than the press left its payload in the stack, where it was then read as the next event type; and the multi-touch release passed the x array as both coordinates. Modality blocks input in core rather than in the ports, so a modal window behaves identically everywhere whether or not the platform implements its own; ports still set the native flag for correct focus and taskbar behaviour. showModal parks the caller through invokeAndBlock exactly as a modal Dialog does, so every other window keeps painting. All 4790 core unit tests pass. Two of them reach into the paint queue by reflection and were updated for its move onto PaintSurface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the two assumptions in the JavaSE port that there is exactly one canvas, which is what stands between it and a second rendered window. getGraphics(Object) fell through to canvas.getGraphics2D() for any screen graphics, so a secondary window would have drawn into the primary window's buffer. NativeScreenGraphics now records the canvas it belongs to and resolves through that. isScreenGraphics(Graphics2D) was literally an identity comparison against the primary canvas's buffer. It is now a membership test over the registered screen buffers. This matters because drawNativePeerImpl uses it to decide whether to undo the zoom scale, so answering wrongly for a second window would mis-scale its peer components. The registry is maintained at the only three places C.g2dInstance is written -- created in getGraphics2D, discarded in createBufferedImage and in the size change reset -- so it cannot drift. Behaviour with a single window is unchanged: the primary canvas is still the owner of its own graphics, and the membership test still answers true for exactly the buffer the identity comparison used to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first real port implementation, and the one that decides whether the design holds. Each Codename One Window becomes a JFrame containing its own instance of the port's existing C canvas, so a second window inherits the whole buffered blit machine -- including the blitCounter aliasing fast path, which is already per-instance state -- with none of it duplicated. Input is tagged at the source: C carries the window id it renders and its listeners dispatch through the window-aware entry points, so an event reaches the right hierarchy without a lookup on the AWT thread. Window id zero routes to the main surface, so the primary canvas keeps its exact previous behaviour. Monitors come from GraphicsEnvironment, with the work area taken from the screen insets so a window centres or maximises without landing under the task bar or dock, and the backing scale from each GraphicsConfiguration's default transform rather than one global retina scale. A window that is dragged onto a display with a different scale raises a monitor-changed event, which is what lets the framework re-lay it out instead of leaving it blurry. Multi-window reports unsupported while a phone skin is loaded, reusing the predicate isFullScreenSupported already applies: a skin simulates one device screen with its own coordinates and zoom, and a real operating system window inside that simulation is incoherent. Headless likewise. Also qualifies java.awt.Window in SourceChangeWatcher, which wildcard-imports both java.awt and com.codename1.ui and so became ambiguous the moment com.codename1.ui.Window existed. A repo-wide scan found no other collisions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds TestWindowManager, a window manager with no operating system behind it, and wires it into TestCodenameOneImplementation as an opt-in. It defaults to absent, so the unsupported platform every mobile port reports is also the default a test sees, and the throwing path is exercised without arranging anything. Its monitor table is scriptable, which is what makes per-monitor DPI testable at all: DesktopMonitorTest describes a 2x laptop panel with a dock reserved at the bottom next to a conventional external display, then asserts that a window picks up the scale and density of whichever one it sits on and that moving between them marks its preferred sizes stale. Getting that wrong is what produces a blurry or mis-sized window, and it would otherwise need a second physical display to catch. WindowTest covers the rest of the contract: constructing a Window on an unsupported platform throws rather than degrading, Desktop still answers safely there, show creates exactly one native window, dispose releases it and is idempotent, title and bounds reach the native window, close honours the close operation and can be vetoed, chrome and modality reach the peer, and each window gets its own id since events are routed by it. Two assertions are the load-bearing ones for the chosen design: a component in a Window resolves that Window through getTopLevelContainer(), and getComponentForm() returns null for it -- while a component in a Form still resolves both. 4808 core unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ates SpotBugs is a zero-findings gate and this change tripped ten. Fixing them properly rather than excluding them turned up a real gap. Five were unused fields on Window -- the press coordinates, the press token and the dragged component. They were unused because Window had no pointer dispatch at all: Container does no hit testing of its own, Form does that work itself, so without it a press inside a window never reached the component under it. Window now performs the same walk Form does, minus the title area and menu bar special cases it has no equivalent of, and implements the Container hooks that expose the press state -- which is what the migrated drag and scroll code in Component reads. One was a naked notify in dispose(). The flag showModal parks on is now published under the very monitor the waiter is blocked on, with a separate flag guarding re-entry, so the wake is tied to the state change rather than being incidental. Four were anonymous Runnables in Display retaining their enclosing instance. They are now one named static WindowCallback. SpotBugs, PMD and Checkstyle are clean over core-unittests; 4808 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were parented to the primary canvas unconditionally, so a BrowserComponent or a text field inside a desktop Window would have appeared on the main window instead of the one containing it. Peer.addNativeCnt now resolves its frame through the owning window at attach time rather than at construction: a peer is created before it is added to a hierarchy, so its window is not knowable when the Peer object is built. editString attaches the Swing editor to the owning window's canvas, and stopTextEditing removes it from whichever canvas it actually landed on rather than assuming the primary one. Both resolve through Display.getWindowPeerForComponent, which walks the component's top level -- so a component on the main form still gets exactly the previous behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the native Windows port. Each Codename One Window is a slot in a new native table with its own HWND, ID2D1HwndRenderTarget and CN1Graphics. The main window is deliberately left out of that table. It stays in cn1Win with its existing HWND, render target and graphics untouched, so the single-window path -- which every existing app and every screenshot baseline exercises -- cannot change behaviour. Secondary windows also get their own window procedure rather than sharing the main one, which is full of main-window-only cases. Window identity in that procedure comes from GWLP_USERDATA set in WM_NCCREATE: O(1) and lock free, which matters because it runs on the pump thread while the EDT is drawing. Events carry the framework's window id, which the native side stores at creation and echoes back, so routing needs no lookup. Creation and destruction marshal to the pump thread through a new WM_CN1_DESKTOPWINDOW, following the blocking SendMessageW pattern the native edit control and file dialog already use -- a window must be created on the thread that owns the message loop. Everything else is legal cross-thread and runs directly. The message loop itself needs no change: GetMessageW already pumps every window owned by the thread. Two things carried over deliberately from the main window because getting them wrong is subtle: D2D1_PRESENT_OPTIONS_RETAIN_CONTENTS, since Codename One repaints only the dirty region and relies on the rest surviving the present; and recording a resize for the drawing thread to apply between frames rather than resizing the render target from the pump thread, which presents black. WM_DPICHANGED honours the rectangle Windows suggests and reports the monitor change, which is what keeps a drag between mixed-DPI displays from leaving the window the wrong physical size. Monitors come from EnumDisplayMonitors with the work area from MONITORINFO, and per-monitor DPI from GetDpiForMonitor resolved dynamically since shcore.dll only exists from Windows 8.1. WM_DESTROY on a secondary window deliberately does not PostQuitMessage: closing a tool window must not exit the application. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the native Linux port. Each Codename One Window is a slot in a new native table carrying its own GtkWindow, GtkOverlay, GtkDrawingArea, GtkFixed peer layer and cairo back buffer. The main window keeps its own file statics in cn1_linux_window.c and is not part of that table, so the existing single-window path is unchanged. Routing is essentially free here, which is the nice part of GTK: every signal handler already takes a gpointer closure, so passing the window struct as the closure data makes each handler window-scoped with no lookup and no shared state. gtk_main_iteration already services every window in the process, so the loop needs no change either. Events carry the framework's window id, stored at creation and echoed back. The delete-event handler returns TRUE so GTK does not destroy the window: Codename One decides, because an application may veto the close from a listener. The window's back buffer sets isWindowTarget, which turns on the #5273 clip clamp -- a clip set while a component paints is confined to the region about to be flushed, so an oversized fill cannot leave stale pixels on the persistent cairo surface. GTK is not thread safe, so every entry point marshals to the GTK main thread through cn1LinuxRunOnMainAndWait, which the port already uses for exactly this. Monitors come from GdkDisplay, with the work area from gdk_monitor_get_workarea. Scale reports GTK's integer scale factor, since that is what actually governs how the toolkit renders, while dots per inch is derived separately from the monitor's reported millimetre size -- the integer factor is far too coarse to describe a display's real resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the Mac Catalyst slice. A Codename One Window becomes a UIWindowScene, with the whole implementation inside #if TARGET_OS_MACCATALYST so the object file an iPhone or iPad build produces is empty and the plain iOS binary is unchanged. Unlike the other desktop ports, the window's content is rendered into a mutable image and the finished raster is assigned to the scene view's layer, rather than the window owning a second Metal surface. That is a deliberate trade: the render path caches its device, pipeline state and glyph atlas against the single rendering view, and making those per-scene is a large refactor of the hottest code in the product, without ARC. The scene still owns a real UIKit view hierarchy, so native peers and native text editing work normally inside a window -- only the drawing arrives as a bitmap. Multi-window is opt-in through a new macNative.multiWindow build hint. That is not caution for its own sake: the existing comment in IPhoneBuilder records that turning UIApplicationSupportsMultipleScenes on changed Catalyst windowing and crashed the screenshot suite with a 26 GB signal loop. The hint now gates both that Info.plist key and IOSImplementation.getWindowManager(), so the key and the API that requires it are switched by the same flag and cannot disagree. Scene arrival is asynchronous, so a created window claims the next scene the delegate receives; the delegate hands it over before installing the main root view controller, and only the application's own scene falls through to that. Teardown releases the scene, window, controller, view and title on the main queue after UIKit has finished with them, since this port has no ARC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The part of the test story that actually demonstrates windowing. A picture of a window proves nothing; these re-run representative UI INSIDE a real operating system window and compare that window's own capture against its own baseline. WindowHostTest hosts content in a Window at three sizes -- 400x300, 900x700 and a deliberately non-square 1000x400 -- and captures through Window.capture() rather than Display.screenshot, because the ordinary path can only see the application's main framebuffer and a second window simply is not in it. The three sizes are the point: a window still measuring itself against the main display would produce three near-identical goldens. The cases were chosen for what fails silently rather than for coverage count. Layout proves sizing and theming resolve against the window. Scroll proves the scroll path, which goes quiet rather than throwing if a component cannot resolve its top level. Graphics exercises the port's pipeline on a non-primary render target with shapes that deliberately reach the edges, where a wrong clip clamp leaves stale pixels. Editing covers native text input, which used to attach the platform editor to the main window's canvas unconditionally. Overlay covers the layered pane that Sheet, InteractionDialog and ToastBar attach to. Modal captures the BACKGROUND window while a modal is up, which is the state that would be blank if the nested event loop had stopped servicing it. MultiWindowApiTest is the behavioural half: no screenshot, runs everywhere, and asserts against what the port reports rather than pixels. Where there is no windowing system it asserts the opposite -- that the capability query says so and that constructing a Window throws rather than degrading. The suite skips without emitting a golden where windows are unsupported, so mobile baselines never contain a picture of something the platform cannot do. The new tests are recorded as not-run in every stored port report, which is honest: CI has not executed them on those targets yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a Desktop Windows chapter next to Desktop Integration, covering the whole feature: where windows exist and where they throw, the Form/Window relationship through TopLevelContainer, lifecycle and close vetoes, chrome and the two coordinate systems, modality, monitors and per-monitor DPI, events, peers and native editing, and the Mac Catalyst opt-in. Two things are called out rather than buried, because they are what will actually catch someone out. getComponentForm() returns null inside a Window, and the failure mode is silence rather than an exception, since most code guards on null and quietly does nothing -- so a component that will not scroll or focus in a window has a named cause. And Catalyst multi-window needs the macNative.multiWindow build hint, because a second window is a second scene and that requires a process-wide Info.plist key. Vale reports zero issues at suggestion level, LanguageTool zero matches across the guide, and the paragraph capitalization check passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a port-level test for the riskiest edit in this work, which had no coverage before and sits in the paint path where a regression shows up as wrong pixels rather than an exception. Two canvases must resolve to two distinct screen buffers -- sharing one is exactly what would make a second window draw into the first window's pixels. And isScreenGraphics has to answer true for a secondary window's buffer as well as the primary one, but still false for a mutable image: drawNativePeerImpl uses that answer to decide whether to undo the zoom scale, so a wrong answer mis-scales a window's peer components. 222 JavaSE port tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compiling CN1MacWindows.m against the actual Mac Catalyst SDK -- which the earlier commit never did -- turned up three defects that would have shipped. Scene-to-window matching was a race. Creation returns a slot immediately and requests the scene asynchronously, and the arriving scene was handed to the first unattached slot. Two windows opened in quick succession could therefore swap identities. Scenes are delivered in request order, so the pending slots are now a FIFO, enqueued on the same main-thread turn as the request; a window destroyed before its scene arrives leaves the queue. The presented frame was a use-after-free waiting to happen. flushGraphics allocates a local Java int[], and the native side wrapped that pointer in a CGBitmapContext, then used the resulting image on a later main-queue turn -- by which time the array is garbage and the collector may have reclaimed or moved it. The pixels are now copied, and handed to a CGDataProvider with a release callback rather than a bitmap context: CGBitmapContextCreateImage is copy-on-write, so it is not defined when the backing buffer becomes free to release, whereas the provider makes that lifetime explicit. The alpha format was wrong. getRGB returns straight ARGB and the image declared kCGImageAlphaPremultipliedFirst, which would darken every pixel that is not fully opaque. A window's content is opaque, so it now skips the alpha channel. Also uses slotForScene, which was dead code, to reject a scene that was already adopted. Verified by compiling both CN1MacWindows.m and CodenameOne_GLSceneDelegate.m for arm64-apple-ios-macabi against the real SDK: clean with -Wall. The same file built for plain iOS exports zero CN1MacWindow symbols, confirming the whole implementation compiles out and the iOS binary is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compiling the native sources -- which the port commits never did -- turned up three defects, one of them serious. WM_CN1_DESKTOPWINDOW was defined as WM_APP + 24, which WM_CN1_WIDGET already uses. Widget ops and desktop-window ops would have been delivered to each other's handlers, both of them casting the same LPARAM to a different struct. Moved to WM_APP + 25; the duplicate is now checked for rather than assumed absent. The two COM release calls in the Windows window layer did not resolve. This port compiles its Direct2D translation units as C++ and resolves COBJMACROS-style call sites through an explicit shim in cn1_windows_comc.h, which defines only the methods the port actually uses -- and it had no Release entry for either the HWND render target or the solid colour brush. Added both, in the shim's existing style, rather than reaching around it. On Linux, the GtkWidget-typed accessors were declared in cn1_linux.h. That header is included by translation units that have no GTK on their include path, and declaring a GtkWidget* there breaks them. Moved to cn1_linux_gfx.h, which is the header that includes gtk and where the equivalent existing declarations already live. Verified with the real toolchains available here: cn1_linux_desktopwindow.c is clean under -Wall against GTK 3, and every Windows translation unit including the new one now reports zero errors of its own. The remaining diagnostics in both ports reproduce identically on master and come from compiling Linux and Windows sources on a Mac. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ault Findings from actually building and running the Catalyst app on a Mac, which no earlier commit had done. MacWindowManager never implemented capture(), so it inherited the base class's null. Every windowed screenshot test failed with "Window capture returned null". On this platform the window's content is already rendered into a mutable image, so a capture is that raster. The screenshot harness waited a fixed 1.2s on a UITimer bound to the current form. Catalyst creates its window asynchronously -- it asks the system to activate a scene and is handed one back later -- so a fixed delay is both too long on the fast ports and too short here, and the timer's bound form is not the window anyway. It now polls for the window actually being renderable, re-queuing through callSerially rather than sleeping: the paint that makes it renderable happens on that very thread, so blocking there would stop the condition ever becoming true. macNative.multiWindow now defaults to false for the sample as well. That is measured, not cautious: with multiple scenes enabled, this suite's OrientationLockScreenshotTest captures its landscape frame and then times out after 20s trying to restore portrait. Catalyst treats a multiple-scene app's windows more like Mac windows and honours orientation requests less, so the regression belongs to the Info.plist key rather than to the window code. This gives the warning already in IPhoneBuilder a concrete mechanism instead of folklore. What the run did confirm: the Info.plist key is emitted correctly, CN1MacWindows.m compiles clean under Xcode's own flags, the app boots with multiple scenes enabled and runs all 178 tests without the crash the older comment described, and MultiWindowApiTest passes on the supported path -- so a real Catalyst Window is created, registered, resolves getTopLevelContainer() to itself, reports null from getComponentForm(), reports its monitor and scale, lays out to its own size rather than the display's, and deregisters on dispose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two corrections from further runs on real hardware. The previous commit blamed multiple scenes for OrientationLockScreenshotTest timing out while restoring portrait. That was wrong. With the key still enabled the test passed in the following runs, so it was a slow-machine flake -- the machine was compiling at the time -- not a consequence of the Info.plist key. The hint stays off by default anyway, on the honest grounds that it changes Catalyst windowing process-wide and an application should opt into that rather than have it changed underneath it. The screenshot harness was also asking the wrong question. It waited for the window to report itself showing at its requested size, but a window reports the size it was asked for before the platform has actually produced anything -- on Catalyst the scene arrives asynchronously -- so both were true within milliseconds and the capture then failed. Readiness is now "a capture succeeds", which is exactly the condition the next line depends on and is correct on every port. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the Catalyst suite showed every windowed screenshot emitting a blank frame: the sizes differed correctly per window, but the content did not, and the harness reported the captures as duplicates of each other. The cause is that a window's raster exists from the moment it is shown, so a capture taken before the first paint cycle returns an empty frame of the right size rather than failing. The harness had no way to tell the two apart. Window now records when a paint cycle has completed and exposes hasPaintedOnce(), and the screenshot harness waits on that as well as on the capture succeeding. This is useful beyond the tests: any tooling that wants a window's content rather than its dimensions needs the same distinction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Built and ran the conformance suite as a Mac Catalyst app on real hardware with multi-window enabled: 0 failures across all 178 tests, and all 14 windowed screenshots captured with distinct hashes and no duplicates -- including the modal case, whose background window is non-blank while a modal is up, which is the property that proves the event loop keeps servicing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The extra macNative.multiWindow switch existed only because Mac Catalyst scenes were unverified. They are verified now -- the whole conformance suite runs as a Catalyst app with multiple scenes enabled -- so gating it behind a second opt-in only meant CI never exercised the feature. UIApplicationSupportsMultipleScenes is a process wide Info.plist key, so it is still keyed off macNative.enabled rather than set unconditionally: that key is true for the Mac Catalyst slice only and false for iPhone and iPad builds, which keeps the iOS output byte for byte identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inspecting the Mac Catalyst captures rather than only their hashes showed three defects that distinct hashes had hidden. A Catalyst scene was never asked for the geometry the window was created with, so the system handed it the main scene's size. The window then laid out into a raster that did not match the request: several captures came out at the main display size with the window's content in the corner. The scene now requests the pending geometry as soon as it connects, and both that request and setBounds convert Codename One's pixels to UIKit's points. getBounds reports pixels to match getWidth and getHeight. The readiness probe accepted a window that had painted and could be captured, neither of which implies the size settled -- which is how the mismatch reached a golden in the first place. It now also requires the window and the captured image to be exactly the requested size, so a platform that cannot grant it fails loudly instead of baking a wrong baseline. A window used its own Window and WindowContentPane UIIDs, which no theme written before desktop windows existed defines, so it painted nothing and came up black. A window is a top level surface, so it now takes the Form, ContentPane and TitleArea styles every theme already has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![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
Dispatch key events to the window's focused component
When f is a Window, this invokes the inherited Container.keyPressed(), because Window does not override the key handlers. That implementation only forwards to a container lead component, so ordinary focused controls receive no physical-key input, focus traversal never runs, and the listeners stored by Window.addKeyListener() are never fired. Window needs form-equivalent key pressed, released, repeated, and long-press dispatch.
ℹ️ 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".
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
The guide gate requires every source block to come from a tagged fixture under a compiled source root, so the snippets are checked by javac rather than only by eye. This chapter had them inline. Two of them did not survive the move as written: one relied on an ellipsis inside a switch and another on a call that has no declaration, so both are now complete code. Also documents the styling a window starts out with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closing one window and opening another failed on Mac Catalyst with "scene invalidated before create completion": the system does not hand out a scene session while a previous destruction is still in flight, and the window that asked was left without one. That is an ordinary sequence, so a closed window now parks its scene for the next window to adopt rather than destroying it. The size query also answered with the size that was requested while the scene did not exist yet, so a window looked correctly sized during exactly the interval when nothing was known about it. It now answers zero until there is something real to measure, and show() keeps the requested size until a port delivers a real one instead of collapsing the window to nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
All three are defects in the previous two rounds' fixes. Size the input tables for 32 secondary windows plus the main surface. I matched CN1_MAX_DESKTOP_WINDOWS exactly last round and forgot the main surface holds a permanent entry of its own, so only 31 secondary slots were usable and the last window the ports allow still lost its drag state. Do not reclaim a drag ring a newer gesture has taken. A release handler can enter invokeAndBlock, whose nested loop dispatches a fresh press in the same window; the reclaim added last round then freed the ring that press had just been given. It is skipped when a press target is recorded for the window again. Tear the pressed state down by gesture rather than by window, for the same reason: the cleanup erased the replacement gesture's target and press token instead of the released one's. It is keyed on the press token now. Dispatch stylus events before the consumable pointer listeners. Form dispatches device-specific events first; I added the stylus dispatch below the listener blocks, so a window listener that consumed a pen press silenced the component's stylus listener. This is the same ordering mistake as the context menu one fixed last round -- I corrected that ordering without checking the stylus dispatch I had added alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38b87d323
ℹ️ 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".
Finish an activated drag when it is released in a window. The release path called pointerReleased for every target, but Component hides the component when a drag activates and only dragFinishedImpl restores it, clears the top level's dragged component and runs the drop callbacks -- so releasing an activated drag left the component invisible and the drop never happened. Both the ordinary and the listener-consumed paths finish it now, as Form does. Capture the releasing gesture's target before the listeners run. The token added last round guarded the teardown but not the target lookup, so a listener that entered invokeAndBlock and had a fresh press dispatched underneath it caused the outer release to fire pointerReleased at the *replacement* gesture's component -- activating it with no native release of its own. Both are the same underlying error as the last round's: the release path was written as "operate on whatever the window's fields say now" when it has to be "operate on the gesture that is ending". The token, the target and the drag now all come from the same captured gesture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dae938745
ℹ️ 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".
Cancel a window's key timers when modality rejects its press. keyPressedImpl arms the repeat and long-key timers before the modal filter has had a say, and the paint loop fires keyRepeated and longKeyPress directly without consulting it again, so holding a key could drive a component behind a modal that never received the press. This is the same defect I fixed for the pointer several rounds ago and did not check for the keyboard at the time; the filter now cancels both. Keep the original press target when a key repeats in another window. The native ports forward every autorepeat as another press, so recording the repeat's window replaced the target and sent the eventual key-up to whichever window had focus by then -- leaving a fire-key-activated Button stuck down in the window that actually saw the press. Also fixes the PMD UnnecessaryFullyQualifiedName that turned build-test (8) red: a fully qualified PointerEvent reference from the context-menu fix two rounds ago, now an import. I had verified that change with the unit suites and the source gates but not with the PMD gate, which is where it failed. Running it locally now confirms clean. Both tests were confirmed to fail without their fix. The first version of the key-timer test passed either way, because the key code it used cannot arm a repeat at all -- it needed a non-negative code to test anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5486333a8
ℹ️ 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".
grabAnimationLock returned animMananger.isAnimating(), which inverts the contract: a caller acquired the "lock" precisely when something else was already animating, and was refused when the window was idle. FloatingHint guards its animation on that return value, so in a window it would animate only during someone else's animation and never otherwise. releaseAnimationLock passed null to flushAnimation. That throws immediately when nothing is animating -- flushAnimation calls r.run() on the null -- and defers the same NPE to the event dispatch thread when something is, by queueing it for updateAnimations to invoke. So it threw unconditionally, one way or the other. Both now mirror Form: a plain boolean lock. Also re-indents the body of handleEvent's try block. The try/finally that scopes the drag-history selector was added around the existing body without re-indenting it, which turned build-test (8) red on Checkstyle IndentationCheck. Worth recording how that got through: mvn verify on core-unittests does produce checkstyle-result.xml, but it exits 0 whether or not there are errors -- the gate is generate-quality-report.py reading the XML. So a green exit code proves nothing and the reports have to be read. All three now show zero: 0 Checkstyle errors, 0 PMD violations, 0 SpotBugs instances. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16556104a8
ℹ️ 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".
Cancel the key repeat that the press armed, not the one the release names. The timer is armed by the press and cancelled from the key-up path, and focus can move between the two -- so releasing a key held in another window cancelled the wrong slot and left the pressing window firing keyRepeated every 10ms with the key physically up, indefinitely. Cancellation is keyed by key code now, which identifies the physical key that armed it regardless of which window has focus. Keep checking outer modals for an unrelated modal window. Reaching a window's own entry returned immediately, which exempted a modal from every other modal rather than only from the ones that own it -- so an unrelated modal shown while an application modal was up accepted input that application modality is meant to stop. The self entry is skipped and the scan continues, with the nesting exemption applied to the owner chain. Two notes on the tests, both found by running them against the un-fixed code. The first modal test asserted the wrong thing -- that a modal nested inside A stays usable while an unrelated application modal B is also up. It does not and should not: B blocks everything outside its own chain. The code was right and the test was wrong. And the repeat test passed either way at first, because its reflection helper compared against window id 0 while Display keys the main surface as -1 so that 0 can mean "unused"; the long-press helper had the same latent flaw and is fixed with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0bee02355
ℹ️ 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".
Record the press target before calling pointerPressed rather than after. A pressed callback can enter a nested event loop -- showModal() does -- and the matching physical release can be processed inside it; with the record made afterwards that release saw no accepted press and was discarded, and the record then landed stale, latching the component and misrouting the next release. Both the single and multi pointer branches. Two more gaps found by diffing Window against Form rather than waiting for them to be reported: isSingleFocusMode returned a hardcoded false while Form computes whether there are fewer than two focusables. Single focus mode changes key handling -- with one focusable there is nothing to traverse to, so the arrow keys belong to the component -- so a one-control window behaved differently from the identical Form. isRevalidateFromRoot returned a hardcoded true, ignoring the Form.revalidateFromRoot property Form honours, so an application that had turned it off got one behaviour in its forms and another in its windows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2f98c5a8e
ℹ️ 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".
The two reported gaps -- setMinimumWindowSize and setDecorated -- were both
instances of the same thing: MacWindowManager implements a fraction of the
WindowManager SPI and the rest silently resolves to the interface's no-op
defaults, so the Java state changed and the window did not. Rather than
fix the two that were reported, I diffed the SPI against all four ports.
Catalyst was missing ten methods the other three all implement.
Implemented here, because Catalyst can express them:
setMinimumSize recorded on the slot and applied to the scene's
sizeRestrictions, including for a scene adopted later
setDecorated hides the title bar's title and toolbar
Documented rather than implemented, because Catalyst cannot express them:
setAlwaysOnTop, setUtilityWindow, minimize, restore and toggleMaximize have
no public UIKit equivalent for a UIWindowScene -- AppKit owns that
behaviour. setModal is a no-op by design, since modality is decided by the
framework and enforced through setInputEnabled, which this port does
implement. setPaintDirtyRegionClip is an optimisation Java SE also omits.
They are listed in MacWindowManager's class documentation and in the
developer guide, so an application knows which controls the platform may
decline rather than discovering it one report at a time. setDecorated is
recorded as partial there too: the frame stays, only the title bar content
is hidden.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 591a275243
ℹ️ 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".
…stener Capture the pointer type, pressure and tilt before a Catalyst window's touch is queued, as the main surface's handlers do. Without it the queued event carried the defaults, so a pen read as an ordinary touch -- which meant the stylus dispatch added a few rounds ago could never fire in a Catalyst window no matter what the Java side did. The Java half was necessary and not sufficient. Release the canvas's global gesture listener when a Java SE window is disposed. The canvas registers an AWTEventListener with the Toolkit for the magnification wheel fallback and the Toolkit holds it for the life of the VM, so disposing only the frame leaked the canvas and its whole hierarchy, and left it inspecting every wheel event in the application -- once per window ever opened. disposingAWindowReleasesItsGlobalGestureListener covers the leak. My first attempt to verify it against the un-fixed code was invalid: the edit I made to disable the cleanup broke the file's brace structure, so the run failed to compile rather than failing the assertion. Redone properly, it reports the listener count staying at 2 instead of returning to 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eca52f5202
ℹ️ 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".
Cancel a window's input timers when it is hidden. A key handler can hide its own window, and a hidden window stays registered -- so the repeat and long-key timers armed by the press that got there kept firing into a component tree the user could no longer see, and kept the event dispatch thread awake. The key-up may never arrive either, once the native window has lost focus. New Display.windowInputCancelled clears that window's key and pointer timers and recorded presses without deregistering it. Recheck modality on every queued wheel step. A wheel gesture is played as four steps queued on the event dispatch thread; the modality check ran once, before an unconsumed wheel listener had the chance to show a modal. The synthetic press, drags and release then scrolled or activated content behind it. wheelRoot resolves through a modality-aware lookup now, so a modal shown mid-gesture stops the remaining steps. Leave placement alone when only the size is set. setWindowSize routed through setWindowBounds, which marks the position as chosen -- so the placeholder (0,0) reached every port as an application-selected location and suppressed the window manager's own placement. Before the peer exists it now updates only the pending dimensions, which is what the method documents and what the guide's own example relies on. Answer a pending window's monitor from the location it asked for. With no peer the lookup fell back to the primary monitor and cached it, so centerOnDesktop() recentred a pre-positioned window back onto the primary display and a pre-show getMonitor() left scale and density stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2d7719d49
ℹ️ 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".
Both findings are consequences of the previous round's fixes. Let a wheel gesture finish once its press has been dispatched. Rechecking modality on every one of the four queued steps -- which is what I added last round -- suppressed the later steps including the only release, so a gesture whose press had already landed never completed and left the top level's pressed and drag bookkeeping stranded. The root is resolved once, by the step that dispatches the press, and the rest follow it. A gesture blocked before its press still never starts, which is the case modality is there to stop. Cancel the component's gesture state before dropping the records. The hide cleanup added last round forgot the recorded targets, which is what stops the runaway timers, but a component that had taken a press -- a Button in STATE_PRESSED from the fire key or a pointer press -- then had no way to learn the gesture ended, and was still latched when the window was shown again. dragInitiated is the existing "ended without completing" primitive: it resets the pressed state without firing the action, which is what a window the user can no longer reach should do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bfa9fd117
ℹ️ 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".
The reported P1 was a fourth instance of one defect: a window that goes away mid-gesture leaves timers running and components latched. I had fixed hide() last round after the third report; native minimization arrives through hideNotify() instead and bypassed it entirely. Rather than patch that path too, the cleanup is now one method called from every path that makes a window unreachable -- hide(), hideNotify() and dispose() -- and it undoes all three kinds of state together: an activated drag and drop, whose component Component has already hidden and which only dragFinishedImpl restores. dragInitiated does not reset drag state, so the previous cleanup left the component invisible and still marked as dragging. It is finished outside the window so no drop target is found: the user never completed the drag, the window went away. a pressed component, latched with no release coming, reset through dragInitiated so the state clears without firing the action. the framework's recorded targets and timers. Also guards the window's press focus on isScrollWheeling(), as Form and LeadUtil both do. dragWheelStep disables only the deepest hit component, so a focusable lead parent still took focus and merely scrolling over a lead-based control stole the keyboard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ee10ea315
ℹ️ 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".
Two more entry points into the defect the previous commit generalised. Losing focus to another application is the fifth way a window stops being reachable, after hide, minimize, dispose and modal blocking. The physical key-up goes to whatever has focus now, so a held key repeated into the old window for as long as it stayed open and its pressed component stayed latched. The focus-lost callback runs the same cancelPendingInput. Cancelling at the transition cannot close the whole race on its own, because packets queued before it are already in flight: dispatching one into a hidden window re-latches the component the cleanup just unlatched, with no release coming. User input aimed at a window that is not showing is now rejected by the same filter that rejects modal-blocked input. The second test took two attempts. The first asserted on the repeat timer and passed either way, because the timer is armed when the packet is queued rather than when it is dispatched -- so the observable damage from a late packet is the component's state, not the timer's. Retargeted at the component, it fails without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5707f6d00a
ℹ️ 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".
Two P1s, and both are the same defect I fixed for Linux in the twenty-second round without checking the other ports: the native editor resolves its host and its lifetime through getComponentForm(), which is null inside a Window by design. Windows parented the EDIT control to the main HWND, so the editor appeared over the main window while the user typed into a secondary one. The owning window's slot is threaded through editStringAt now, as it already is on Linux. Catalyst rejected the request outright: editString returned as soon as getComponentForm() was null, so the port that advertises window support could not edit any text field in a window at all. The editing path resolves the top level instead -- for the asyncEditing client property, for the frame tracker, and for the multiline revalidate. Bottom padding editing mode stays Form-only, since it exists for a virtual keyboard a desktop window does not have. Auditing all four ports rather than the two reported found the same defect a third time, unreported: both Windows and Linux bound the editor's polling timer to the Form, so inside a window no timer ran, the native control's text was never streamed back into the field and the edit never auto-committed. That needed a UITimer that can bind to any top level, which is added here. Also lets a Catalyst minimum size be cleared: the SPI expresses "no minimum" as non-positive dimensions and the live path only handled positive ones, so a cleared minimum silently stayed in force. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a56bbfc77
ℹ️ 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".
The Java guard I removed last round let a Catalyst window's text field reach the native editor, but the native side still added the editor to CodenameOne_GLViewController's view unconditionally -- so it appeared on the main surface while the user typed into a secondary window. Half a fix: correct on the Java side of the port boundary and inert on the other, which is the second time that shape has come up here. The window being edited is published to the native side before the call rather than threaded through the twenty-odd argument editStringAt bridge. There is one native editor at a time, so a single slot is enough. The attachment point is chosen under TARGET_OS_MACCATALYST, leaving the iOS path exactly as it was. Also fixes the UITimer overload added last round: a one-shot deregistered itself from Display.getCurrent() rather than from the top level it was bound to. Those are the same object for the Form overloads, which is why the line looked right, but a timer bound to a Window was never deregistered and its supposedly one-shot callback fired every interval forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14b81ab0e8
ℹ️ 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".
Peer geometry multiplied by the global retinaScale -- the main display's -- while the surface, backing buffer and pointer mapping around it already used the owning canvas's scale. On a desktop whose monitors have different backing scales, a peer in a secondary window was then offset and sized by the ratio between the two monitors, so a native browser or text editor drifted away from the Codename One component it belongs to. Adds Peer.peerScale() and routes every peer geometry site through it: the backing buffer, the buffer's paint transform, calcPreferredSize() and both onPositionSizeChange() branches. For the main window canvasScale() returns retinaScale by definition, so single-window behaviour is byte-for-byte unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a9b5535e9
ℹ️ 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".
Comment on lines
735
to
+736
| markPointer(key); | ||
| pointerPressed(x, y); | ||
| windowPointerPressed(windowId, x, y); |
There was a problem hiding this comment.
When pointer events with different buttons or device types are pending together, markPointer() overwrites the implementation's single shared metadata record before windowPointerPressed() merely queues the coordinates; Display.handleEvent() does not build the PointerEvent until later (Display.java:3685). The Win32 pump explicitly translates queued bursts before returning (cn1_windows_window.cpp:1002-1004), so a later left-button or mouse event can make an earlier secondary-window right-click or pen event appear to use the wrong button/type, breaking context menus and stylus callbacks. Include the metadata in each queued packet rather than leaving it in shared mutable state; the Linux drain has the same ordering.
Useful? React with 👍 / 👎.
Comment on lines
+415
to
+416
| if (width > 0 && height > 0) { | ||
| p.frame.setMinimumSize(new Dimension(width, height)); |
There was a problem hiding this comment.
When a JavaSE window is on a HiDPI monitor, the SPI supplies this minimum in Codename One/device pixels, while java.awt.Window.setMinimumSize() expects AWT logical units. The same manager explicitly multiplies canvas dimensions by the monitor scale when reporting Codename One sizes (getWidth()/getHeight()), so a requested 320-pixel minimum on a 2x display becomes a 640-device-pixel native minimum and also changes meaning when the window moves between monitors. Divide by the owning canvas's current scale and refresh the constraint on monitor changes.
Useful? React with 👍 / 👎.
Comment on lines
+1144
to
+1149
| gdk_monitor_get_geometry(mon, &r); | ||
| widthMm = gdk_monitor_get_width_mm(mon); | ||
| if (widthMm <= 0 || r.width <= 0) { | ||
| return; | ||
| } | ||
| op->result = (int) ((r.width * 25.4) / widthMm + 0.5); |
There was a problem hiding this comment.
When GTK reports a scale factor greater than one, gdk_monitor_get_geometry() provides the monitor width in application/logical pixels, but widthMm is the physical width. Computing DPI from r.width alone therefore reports roughly half the physical DPI at scale 2; Monitor.getDotsPerInch() is wrong and LinuxWindowManager.getMonitorDensity() can classify a HiDPI display as low density. Multiply the logical width by gdk_monitor_get_scale_factor(mon) before converting it to DPI.
Useful? React with 👍 / 👎.
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.
Codename One has no windowing API. Even on JavaSE, Mac, Win32 and Linux, where the OS has real windows, an app gets exactly one, welded to a single global "current Form":
CodenameOneImplementationholds onecurrentForm,Display.edtLoopImplpaints one surface per tick,paintDirtyuses one global paint queue clipped togetDisplayWidth()/getDisplayHeight(), andhandleEventroutes every input event to one form. Everything that looks like a second window today —Sheet,InteractionDialog,ToastBar,Dialog— is an overlay inside the current form's layered panes.This adds real native windows, each rendering its own component tree, on all four desktop targets, without changing the single-form model mobile depends on.
API
TopLevelContaineris the shared contractFormandWindowboth implement. Its members were chosen by counting actualgetComponentForm().<method>()chains inCodenameOne/src, and every one of them was already public onFormwith an identical signature, soFormneeded nothing beyond theimplementsclause andasContainer()— a Java interface cannot extend a class, so without that bridge aTopLevelContainerreference cannot go anywhere aComponentis wanted.Window extends Container implements TopLevelContainer. Inside a windowgetComponentForm()returnsnull, by design;Component.getTopLevelContainer()is the new resolution API, and core now uses it internally.DesktopandMonitorare the public parallel toDisplayfor "what screens exist and what windows are open", including per-monitor DPI and backing scale;Displaykeeps meaning "the main app surface" exactly as before.Modality is enforced in core rather than per port, so it behaves identically everywhere:
Displaykeeps a modal stack andhandleEventdrops input to blocked windows.showModal()parks the caller throughinvokeAndBlockthe wayDialogalready does, which re-enters the event loop — so every other window stays live and repainting while a modal is up.Implementation
The impl SPI is a single
WindowManagerfacade returned fromCodenameOneImplementation.getWindowManager(). Returningnullis the capability query, so there is no separateisMultiWindowSupported()that could drift from it. Only genuinely universal operations are abstract; anything a port might not offer has a no-op default, so adding a capability later never breaks a port.Per-window paint state moves into a
PaintSurfacevalue object with the main window as instance zero;getCodenameOneGraphics(),repaint(Animation),cancelRepaintandhasPendingPaints()keep their signatures, so every existing port still compiles and behaves.paintDirty()'s body is parameterized rather than globally rebound — a global "active surface" was rejected becauseDisplay.getDisplayWidth()is public and callable off the EDT, so a live binding would change its answer re-entrantly across ~210 call sites.Events pack the window id into the type word (
type | (windowId << 8)). Window 0 is numerically identical to the previous wire format, so drag coalescing and the stack-swap logic are untouched. The port is handed the id at creation and echoes it back, so there is no peer-to-window map on the off-EDT input path.Ports: JavaSE (per-canvas graphics de-singletonization —
getNativeGraphicsused to return one shared instance, andisScreenGraphicswas an identity check against one buffer, so a second window would have drawn into the first window's pixels), native Windows (Direct2D per-window render targets,GWLP_USERDATAidentity,WM_DPICHANGED), native Linux (per-window cairo back buffer, GTK closure data), and Mac Catalyst (UIWindowSceneper window). Peer components and native text editing work in every window on every one of the four. iOS, Android and JavaScript need no port changes at all: they inherit the false capability and the throw lives in core.Latent bug fixed on the way
handleEventreturnedoffsetunchanged when the form was null, while the caller loopswhile (offset < actualTmpPointer)— an infinite EDT spin. It is unreachable today only because all nine entry points guard ongetCurrentForm() != null; window disposal with events in flight makes it reachable. It is now askipEventthat drains the packet so the rest of the batch still dispatches.Testing
Core unit tests drive a scriptable fake
WindowManageronTestCodenameOneImplementation— settable, defaulting to null, so the unsupported path is the default — covering lifecycle, paint isolation, event routing, modality including a modal window nested in a modal dialog, theTopLevelContainercontract, and a fake multi-monitor table at mixed DPI. JavaSE port tests cover the per-canvas graphics resolution, which is the riskiest edit here and had no coverage before.The centrepiece is a windowed screenshot family in
scripts/hellocodenameone: representative UI re-run inside a real window at several sizes and compared against its own goldens. A picture of a window proves nothing; layout, scrolling, graphics, layered overlays, native editing and modality rendering correctly on a non-primary surface is the actual claim. The three sizes, including a deliberately non-square one, are what prove content lays out to the window rather than toDisplay.getDisplayWidth(). This needed per-window capture on every port, since the existing pipeline can only see the main framebuffer.Mac Catalyst was built and run on real hardware for this branch rather than left to CI, because it is the hardest of the four. That found four defects compiling never would have:
capture()was unimplemented; the readiness probe was a false positive; captures were taken before the first paint; and the scene was never asked for the geometry the window was created with, so several captures came out at the main display size with the window's content in the corner.Known scope limits, documented
HTMLComponent, accessibility on secondary windows,Dialog.show()from inside a window and form transitions into or out of one are out of scope for v1 and called out in the guide.Display.getDisplayWidth()/getDisplayHeight()keep reporting the main window; components inside a window use their top level's size.🤖 Generated with Claude Code