Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#13286 closed bug (fixed)

[Screen] Bug- monitor view does not update

Reported by: perelandra Owned by: looncraz
Priority: normal Milestone: Unscheduled
Component: Preferences Version: R1/Development
Keywords: Screen Backgrounds Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

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.

Attachments (1)

0001-Bug-fix-When-both-Backgrounds-and-Screen-are-open-if.patch (6.9 KB) - added by perelandra 2 years ago.
I have built these changes to Backgrounds and Screen under x86gcc2, x86 and x86_64

Download all attachments as: .zip

Change History (14)

Changed 2 years ago by perelandra

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

comment:1 Changed 2 years ago by perelandra

Has a Patch: set

comment:2 Changed 2 years ago by waddlesplash

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

comment:3 Changed 2 years ago by pulkomandy

Owner: changed from nobody to looncraz
Status: newassigned
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.

comment:4 Changed 2 years ago by perelandra

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

comment:5 Changed 2 years ago by 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.

comment:6 Changed 2 years ago by pulkomandy

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

comment:7 Changed 2 years ago by 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.

comment:8 Changed 2 years ago by 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?

comment:9 Changed 2 years ago by 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.

comment:10 Changed 2 years ago by pulkomandy

Resolution: fixed
Status: assignedclosed

Applied in hrev51140.

comment:11 Changed 2 years ago by 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).

comment:12 Changed 2 years ago by 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.

comment:13 in reply to:  12 Changed 2 years ago by 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.

Note: See TracTickets for help on using tickets.