Conversation
…lag fix, VSync page flipping, taskbar clock Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
…ove duplicate fb variable Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
… UTC offset UX Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
…RAM after Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR advances nextOS to 3.0.0 by adding runtime resolution switching (via BGA), improving frame/input ordering for smoother dragging and cursor rendering, and introducing a taskbar clock with persisted UTC offset/resolution settings.
Changes:
- Add BGA-backed runtime resolution switching (
fb_set_resolution()+compositor_set_resolution()) and persist resolution to disk via Settings. - Add taskbar clock (CMOS RTC) with cached reads and a Settings “Clock” tab for UTC offset persistence.
- Adjust main loop ordering to process mouse input before rendering and draw the cursor into the backbuffer before
fb_swap().
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| kernel/ui/compositor.h | Exposes new compositor APIs for resolution switching and UTC offset. |
| kernel/ui/compositor.c | Implements taskbar clock caching, UTC offset handling, resolution switching, and removes cursor draws from mouse handler paths. |
| kernel/gfx/framebuffer.h | Adds fb_set_resolution() API and updates header description. |
| kernel/gfx/framebuffer.c | Implements BGA probing and register programming for runtime mode switching; keeps swap as backbuffer→VRAM copy. |
| kernel/kernel.c | Bumps version/heap size; reorders main loop for smoother input/render/cursor swap. |
| apps/settings/settings.c | Extends persisted settings (resolution + UTC offset), adds Clock tab, and improves mouse speed slider UX. |
| kernel/net/net_stack.c | Updates HTTP/HTTPS User-Agent version string to 3.0.0. |
| apps/browser/browser.c | Updates homepage strings to 3.0.0. |
| uint32_t *new_canvas = (uint32_t *)kmalloc(cw * ch * 4); | ||
| if (new_canvas) { | ||
| for (int p = 0; p < cw * ch; p++) | ||
| w->canvas[p] = 0xF0F0F0; | ||
| new_canvas[p] = 0xF0F0F0; | ||
| if (w->canvas) kfree(w->canvas); | ||
| w->canvas = new_canvas; | ||
| } | ||
| /* If alloc failed, keep old canvas — better than white screen */ | ||
| w->width = new_w; | ||
| w->height = new_h; | ||
| } |
There was a problem hiding this comment.
resize_canvas() updates w->width/w->height even when the new canvas allocation fails and the old canvas is kept. draw_window() uses win->width/height to compute the source canvas dimensions (cw/ch) and will then index win->canvas using the new cw/ch, which can read past the old allocation and corrupt memory. Consider only updating width/height after a successful resize, or tracking canvas dimensions separately so failed resizes keep dimensions consistent with the existing canvas.
There was a problem hiding this comment.
Fixed in dea0dcc — w->width and w->height are now only updated inside the if (new_canvas) block, so a failed allocation preserves both the old canvas and its matching dimensions.
| if (now - clock_last_read >= 1000) { | ||
| clock_last_read = now; | ||
| outb(0x70, 0x0B); uint8_t regb = inb(0x71); | ||
| outb(0x70, 0x04); uint8_t hour = inb(0x71); | ||
| outb(0x70, 0x02); uint8_t min = inb(0x71); | ||
| if (!(regb & 0x04)) { | ||
| hour = (hour >> 4) * 10 + (hour & 0x0F); | ||
| min = (min >> 4) * 10 + (min & 0x0F); | ||
| } | ||
| /* Apply UTC offset */ | ||
| int h = (int)hour + clock_utc_offset; | ||
| if (h < 0) h += 24; | ||
| if (h >= 24) h -= 24; | ||
| cached_hour = (uint8_t)h; | ||
| cached_min = (min <= 59) ? min : 0; |
There was a problem hiding this comment.
CMOS RTC reads here don’t guard against the update-in-progress (UIP) window and don’t handle 12-hour mode/PM bit. If the RTC is in 12-hour format, the hour register’s high bit can indicate PM and the BCD conversion will produce invalid hours (e.g., 0x81 → 81). Consider waiting until register A bit 7 is clear (and/or doing a stable double-read) and normalizing hour based on reg B’s 12/24-hour flag before applying the UTC offset.
There was a problem hiding this comment.
Fixed in dea0dcc — now checks register A bit 7 (UIP) before reading, strips the PM bit (0x80) from hour before BCD conversion when in 12-hour mode, and converts 12h→24h using reg B bit 1.
| int compositor_set_resolution(int w, int h) | ||
| { | ||
| if (fb_set_resolution((uint32_t)w, (uint32_t)h) != 0) | ||
| return -1; | ||
|
|
||
| /* Update mouse bounds to new resolution */ | ||
| mouse_set_bounds(w, h); | ||
|
|
There was a problem hiding this comment.
compositor_set_resolution() casts w/h to uint32_t (and fb_set_resolution further casts to uint16_t) without validating inputs. A negative or very large w/h can wrap/truncate into unexpected hardware register values and lead to incorrect allocations. Consider validating w/h are positive and within the supported/safe range before calling fb_set_resolution().
There was a problem hiding this comment.
Fixed in dea0dcc — compositor_set_resolution() now rejects w/h outside 640×480–1920×1080 before any casts or hardware register writes.
|
@copilot apply changes based on the comments in this thread |
…ation Co-authored-by: dynodevv <85120286+dynodevv@users.noreply.github.com>
All three review comments addressed in dea0dcc:
|
w->width/w->heightafter successful canvas allocation. Failed alloc now preserves both old canvas AND old dimensions, preventing out-of-bounds reads indraw_window().✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.