Repository navigation
fix: draw icons once so soft alpha edges are not doubled - #223
Conversation
An icon-mode lister drew every icon as lister_get_icons() read it, then drew all of them again with backdrop_show_objects(BDSF_RECALC) once the directory was done. IconLib blends the soft edges of alpha icons with what is on the screen, so the second pass doubled them; scrolling redrew the icons once and they changed. Icons shown while reading are now marked BDOF_SHOWN and the final redraw skips them. Any other redraw that does not clear the window first (for example the REFRESHF_ICONS refresh after each command run in an icon lister) also drew icons over themselves. backdrop_draw_object() now clears a borderless icon's area before drawing it, as the DrawIconState() builds already did, unless the caller has just cleared the whole display (BRENDERF_ERASED from backdrop_show_objects() with BDSF_CLEAR).
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: 86c4382707
ℹ️ 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".
Clearing a blending icon's area before drawing it also wiped the parts of icons under or over it, and the redraw after loading skipped icons that had been drawn while loading even when the cleanup's sort had changed their stacking order. - backdrop_render_object() clears the icon's image area, then draws the icons under it, the icon itself and the icons over it, the others clipped to that area. backdrop_erase_icon() redraws the icons it uncovered the same way, bottom first. - backdrop_show_objects() without BDSF_CLEAR first clears every blending icon, then draws all of them in order. - lister_get_icons() puts each new icon at the head of the list, so drawing it on top while loading matches the stacking order. If every new icon has a position and nothing else is in the list, nothing is drawn again; otherwise the window is redrawn after the cleanup. - An icon waiting for its LISTER_SHOW_ICON (BDOF_SHOW_PENDING) is left out of other redraws, so a refresh during loading does not draw it before the message draws it again.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b430c8e7a
ℹ️ 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 DrawIconState() path cleared a borderless icon's area itself unless BRENDERF_ERASED was set, so a new icon drawn on top while loading (BRENDERF_NEW) wiped the icons under it on OS4 and AROS. Every caller now clears before drawing, so that clear is gone. - The clip region for redrawing overlapping icons is built, and its allocation checked, before the area is cleared. Without memory, backdrop_render_object() draws over what is there and backdrop_erase_icon() redraws the covered icons in full, instead of leaving a hole. - BDOF_SHOW_PENDING is cleared under the window lock, in backdrop_render_object() as part of the first draw, so no other redraw can draw the icon between the flag change and that draw.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c6789c694
ℹ️ 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".
Only icons with a position, which are drawn as they are read, go to the head of the list. Icons without one keep their directory order, which the cleanup uses to lay them out; moving them to the head reversed the layout of unsnapshotted drawers.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09305c5d74
ℹ️ 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".
- An icon whose LISTER_SHOW_ICON could not be sent, or was never handled, still has BDOF_SHOW_PENDING after the call. It is counted as not shown, so the window is drawn again at the end. - backdrop_calc_virtual() took the window lock before the backdrop list lock, the reverse of backdrop_show_objects() and the other drawing paths. It now takes the list first.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Icons are drawn once. Before this, an icon-mode lister drew every icon twice when it opened, and every command run in an icon lister drew all its icons over themselves again. With an icon.library that blends soft alpha edges with the screen (IconLib on an RTG screen), each extra pass makes those edges stronger. Scrolling redrew the icons once, so they changed after scrolling.
Redrawing an icon now clears its area first, and the icons that overlap it are redrawn there in their stacking order.
Issue
Reported by Peter Keunecke (IconLib) by mail after #214-#216; no GitHub issue. He saw the doubled edges on PowerIcons' soft-alpha PNG icons (Aminet
util/wb/PowerIcons.lha) and that scrolling removes them for good.Changes
lister_get_icons()sends each positioned icon to the lister as soon as it is read (LISTER_SHOW_ICON), then calledbackdrop_show_objects(info, BDSF_RECALC)at the end, which drew every icon again without clearing. Positioned new icons now go to the head of the list, so drawing each one on top while loading matches the stacking order that later redraws use (the head is drawn last and hit first). Icons without a position keep their directory order, which the cleanup lays them out in. If every new icon has a position and nothing else is in the list, nothing is drawn again. Otherwise the cleanup places and sorts the icons and the window is drawn once more, clearing each icon first.LISTER_SHOW_ICONis still on its way is markedBDOF_SHOW_PENDING; other redraws leave it out until the lister has drawn it, so a refresh during loading cannot draw it before the message does. The flag is cleared under the window lock, as part of that first draw. If the message could not be sent or was never handled, the icon counts as not shown and the window is drawn again at the end.backdrop_render_object()(selection, AppIcon and name changes and so on) clears the icon's image area, then draws the icons under it, the icon itself and the icons over it. The other icons are clipped to that area, so their own edges are not drawn twice. The plainmastercode cleared only on a state change and then wiped the parts of icons underneath (see the table below).backdrop_show_objects()withoutBDSF_CLEARclears the image area of every icon first, then draws all of them in order. That covers theREFRESHF_ICONSrefresh after every command run in an icon lister, desktop refreshes and selecting all. Callers now always clear before drawing, so theUSE_DRAWICONSTATEpath no longer clears borderless icons itself (it would have wiped the icons under a new one drawn on top).backdrop_erase_icon()redraws the icons it uncovered bottom first, clipped to the cleared area, instead of top first and in full. The clip region is built before anything is cleared; without memory for it, Opus draws over what is there rather than leave a hole.backdrop_calc_virtual()took the window lock before the backdrop list lock, the reverse of the drawing paths; it now takes the list first. This also applies to its callers onmaster.Affected platforms
os3)os3-68000)os4)mos)i386-aros)x86_64-aros)aarch64-aros)os3(full clean build) has the same warning count asmaster; the Program builds foros4,mosandx86_64-aroswith no new warnings in the changed files. Onlyos3was run.Verification
Emulated A4000/060, OS 3.2, icon.library 51.4.602 (TC020), ZZ9000 32-bit Workbench, JIT off, edge transparency. Window grabs with
ReadPixelArray(); the reference is a re-read (CLEAR_ONLY, then one draw).31 PowerIcons
Miscicons in a grid:mastervs referenceClearSizes(command refresh)Four PNG icons with saved positions that overlap (two cigarettes with soft smoke, the Boing ball, and one apart):
mastervs referenceClearSizesWork:Audio(50 icons with saved positions), two boots each:master1.167 / 1.172 s per open, this PR 1.011-1.046 s.coverhelper I used there for a "full redraw" opens a window over the lister, but lister windows did not get a refresh from it, so that comparison never redrew anything. Peter's stripes when scrolling slowly fit this bug: each strip that scrolls in is drawn once, next to icons that were drawn twice.Checklist
ChangeLogupdated with a[dopus5 next]entry.documents/DOpus5.guidenot changed: no user-visible option or command.*_REVISIONbump deferred, as for the other[dopus5 next]entries.