Fix excessive redraw from status bar#112
Conversation
| Fl_Box(0, 0, 0, 0), _line(line), _meta(meta), _event_x(0), _event_y(0), _prelude(prelude), _macro(macro), _prefix(), | ||
| _suffix(), _tip(tip_), _warp_id(warp_id), _warp_to(NULL), _prefixed(false), _suffixed(false), _hex_coords(false) { | ||
| user_data(NULL); | ||
| box(OS_NO_BOX); |
There was a problem hiding this comment.
OS_NO_BOX is like FL_NO_BOX except redrawing it doesn't request that the parent window redraws the child region background.
OS_NO_BOX is intended for widgets that definitely draw their entire region, eg with an image, without the need for a box or frame or a background provided by the window.
| mw->update_status(this); | ||
| mw->update_gameboy_screen(this); | ||
| redraw(); | ||
| mw->redraw_map(); |
There was a problem hiding this comment.
The map needs to be redrawn when the mouse enters or leaves an Event, especially when the Event is outside the map.
| mw->redraw_map(); | ||
| return 1; | ||
| case FL_MOVE: | ||
| mw->redraw_map(); |
There was a problem hiding this comment.
I couldn't think of any reason why the map would need to be redrawn for each mouse movement within an Event. If this needs to be added back, let me know.
| _hover_xy->box(OS_NO_BOX); | ||
| new Spacer(0, 0, 2, 21); | ||
| _hover_event = new Label(0, 0, text_width("Event: X/Y ($999, $999)", 8), 21, ""); | ||
| _hover_event->box(OS_NO_BOX); |
There was a problem hiding this comment.
OS_NO_BOX is also used for the 5 Labels inside the _status_bar.
Even though the Labels don't "draw their entire region" like the image buttons, they need to use OS_NO_BOX because otherwise when their parent window is asked to redraw the child background region, that ends up triggering a redraw of other sibling widgets, like the main Workspace+OS_Scroll. This is specifically what was causing the laggy cursor reported in #100.
To avoid this, the Labels will never automatically ask their parent window to redraw their background, but then you must explicitly redraw the parent Toolbar any time a Label's label text is updated. (The Toolbar is now responsible for drawing the background, eg in the Brushed Metal theme.)
| event->redraw(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Because the status bar was always causing constant redraws of the entire map, that hid an existing bug where Blocks are redrawn and then obscure the 0-4 events that overlap on top of that Block.
Now that the constant redrawing is no longer happening, when a Block is redrawn then we need to ensure any overlapping Events also get redrawn.
| Toolbar::Toolbar(int x, int y, int w, int h, const char *l) : Fl_Group(x, y, w, h, l), _spacer(0, 0, 0, 0) { | ||
| labeltype(FL_NO_LABEL); | ||
| box(OS_TOOLBAR_FRAME); | ||
| box(OS_TOOLBAR_BOX); |
There was a problem hiding this comment.
Unfortunately, now that the parent window will not be responsible for drawing the child background region for the status bar Labels, then the Toolbar must draw the background. So this had to be changed from simply a frame to a full box. (This is only super relevant for the Brushed Metal theme.)
| if (d & FL_DAMAGE_ALL) { | ||
| draw_box(); | ||
| draw_label(); | ||
| } |
There was a problem hiding this comment.
Since the Toolbar now has a full box and not just a frame, the box needs to be drawn before the children.
This is the most questionable change in this PR because d can be modified while drawing the children, but now the box is drawn earlier before d can be modified.
This seems OK to me, but let me know if it causes any issues.
I think it's OK because Polished Map doesn't really leverage the "dynamic" nature of auto-packing the widgets inside the Toolbar, except on first draw.
This "auto-packing" is actually annoying enough that I removed it completely in Crystal Tracker here, and now the Toolbar is a lot "dumber" and easier to work with. I'm curious if you really prefer to keep the auto-packing here or if you'd be OK with simplifying the Toolbar like I did.
1 participant
Fixes #100