[Screen] Bug- monitor view does not update

[perelandra]

If both preflets Backgrounds and Screen are open (which is likely since in Screen you can open Backgrounds by clicking on the monitor preview or button) if the workpace background color is changed in Backgrounds the Screen monitor preview does not refresh to get the updated color. This patch adds a message sent from Backgrounds to Screen to inform Screen to refresh the monitor preview.

[perelandra]

I have built these changes to Backgrounds and Screen under x86gcc2, x86 and x86_64

[waddlesplash]

Changes look good to me; unless anyone else has comments I'll apply in the next few days.

[pulkomandy]

printf("setting background image failed: %s\n", error.String()); 

can be removed.

//static const uint32 UPDATE_DESKTOP_COLOR_MSG = 'udsc';
    // This is now defined in headers/private/preferences/ScreenDefs.h 

can be removed.

Reassigning to looncraz in case he has something more to say, he worked on live color updates.

[perelandra]

Any more comments on this? If you want I can push the changes with Pulkomandy's suggested modifications.

[looncraz]

Sorry, I didn't see this become assigned to me somehow (or forgot).

Someone marked B_DESKTOP_COLOR as deprecated, which I think was less than advantageous.

I believe a proper solution would involve piggy-backing on the B_COLORS_UPDATED message while using B_DESKTOP_COLOR. One would need to add a relevant workspace and screen_id to the message, though, which means special handling for this case no matter how it is done.

I think the patch, as it is, is not really desirable. But I always prefer more generic solutions... is it worthwhile broadcasting a color update message to every app? That's a tough question to answer given the cost is still very low.

[pulkomandy]

The problem with B_DESKTOP_COLOR is you can have a different one for each workspace. Could we make this work?

[axeld]

Nope. I'd say B_DESKTOP_COLOR could be used as the default color, though, when the system boots up, or the workspaces settings file is missed/deleted. Other than that, it can be safely removed.

[perelandra]

All this patch does is signal that a desktop color change has occurred, not specifically what color or on what workspace. That triggers Screen to query the app_server for the updated value.

How many applications would use such a message if it was broadcast globally? Anything that provides a preview view of the desktop, but probably not that many. Is there an interface by which an application can "subscribe" to watch for a specific message on the app server? I know you can watch the Roster for when an application quits or launches, anything similar for app_server?

[pulkomandy]

The thing is, there already is another system for live color updates, directly integrated in app_server. Looncraz did most of the work there, adding SetViewUIColor and the like, which lets the app_server automatically update the colors as they are changed.

I was wondering (and I think that's what Looncraz was referring to, too), if we could use this in combination with B_DESKTOP_COLOR, so that the view updates its color automatically. It depends on the meaning of B_DESKTOP_COLOR, wether it is, as axel suggests, the "default" desktop color, or the current setting (as for all the other B_*_COLOR). With the extra catch that the desktop color is settable per workspace, so it is not quite the same as the other colors already.

Well, if B_DESKTOP_COLOR isn't applicable, then the patch sounds fine.

[pulkomandy]

Applied in hrev51140.

[looncraz]

Sorry about not responding earlier - I was on vacation.

Making this work as a global event wouldn't be terribly difficult, though you would be able to use set_ui_color() with B_DESKTOP_COLOR except to have it apply to the current desktop...

BScreen's SetDesktopColor would be the only proper way to set desktop color. It would construct a message to the app_server

BMessage {
    screen_id        screen;
    uint32           workspace;
    rgb_color        color;
}

The app_server would then set the color and broadcast a B_COLORS_UPDATED message with B_DESKTOP_COLOR and the above information (possibly more than one entry, using a custom solver for the DelayedMessage in the app_server - which is already supported).

Screen, Backgrounds, and Tracker would simply parse that message and there would be no need for any custom communications. Third party apps which want to respond to desktop color changes would need only do the same (such as desktop applets).

[axeld]

Can't we simply send a message causing the BWindow::ScreenChanged() hook to be called? That's the closest thing we have, at least.

Another alternative would be to register for desktop changes at some place. If I'll ever finish ​https://github.com/axeld/haiku/tree/background, then BScreen would be the natural owner of this API.

[looncraz]

Replying to axeld:

Can't we simply send a message causing the BWindow::ScreenChanged() hook to be called? That's the closest thing we have, at least.

You could, which would reduce the data sharing with applications somewhat (only saving a few cycles per BApplication and BWindow, though... B_COLORS_UPDATED is very efficient).

Another alternative would be to register for desktop changes at some place. If I'll ever finish ​https://github.com/axeld/haiku/tree/background, then BScreen would be the natural owner of this API.

I've always preferred the subscriber/provider model, myself... considering how few apps will care about the desktop color, this seems like a reasonable area to have custom facilities... but then you're tracking a list of subscribers.

As I said previously, I do prefer generic solutions as a matter of course - even if it has more overhead, but that is not always best.