[haiku-development] Re: Final Set*UIColor Patch, Version 3e
- From: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Mon, 23 Nov 2015 21:25:16 +0100
Am 22.11.2015 um 02:40 schrieb looncraz:
My final patch, located here: http://files.looncraz.net/svc/
Next round of review, still not through, but I think I got all the
important things.
First of all, I think the API changes in BView are a nice addition, and
also nicely done.
I would like to get some discussion going on why you did some stuff the
way you did it.
There are two use cases of UI color changes:
1) one color is changed (via slider in Appearance)
2) all colors are changed as part of a theme change
You only get the update timing issues for case 1), and the usual
solution that one would come up with would be to limit the update
interval from the source.
For 2), the preferred solution would be to be able to set the UI colors
atomically, ie. using a set_ui_colors() method.
The only use for BColorMap client side is that you know which view color
you have to update (instead of just all of them).
However, you still have to invalidate the view, which limits the
usefulness of BColorMap a lot.
I don't see any value in knowing which 23 UI colors the theme changed,
but please enlighten me why this could be useful.
I could imagine that adding AddColor(), FindColor(), HasColor(),
SetColor(), GetColor() to BMessage could be a somewhat nice addition,
although using B_INT32_TYPE would then be questionable. Once we want to
support more than 8-bits per channel, such methods would be nice to
have, anyway, though.
Is there any other use case you anticipate for the delayed message
implementation? If all you really do with it is reply throttling, it
could be solved a lot simpler, especially since you already keep pending
data extra (as I said above, I wouldn't do any housekeeping there, though).
BView: having virtual methods to deal with the color/font updates would
be nice, but it's probably not worth it considering we only have 4 slots
left.
Desktop::ColorUpdated() Why mark the window as dirty? Only the decorator
should be marked dirty -- the window does the invalidate itself already
(if needed). Calling it for each changed color looks very expensive,
anyway (for really little gain).
Why add the UI color stuff to the server's View at all? It looks like it
could be handled client side completely. Have you looked into this?
ServerWindow/invalidate: why not find the view by token (fast lookup)
instead of walking over the complete view hierarchy?
I know you're fond of it, but I really have a hard time to see any real
use case for all this code, though. Why not just use a BMessageRunner
with B_INVALIDATE to your view? I could understand the desire to do this
entirely client-side to safe resources, but not this way.
TabDecorator: there is no "virtually constant"; please use the 'f'
prefix for member variables.
Bye,
Axel.
Other related posts:
- » [haiku-development] Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Axel Dörfler
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Stephan Aßmus
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Dario Casalinuovo
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Jérôme Duval
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Jérôme Duval
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- scottmc
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- scottmc
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Axel Dörfler
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Jérôme Duval
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Adrien Destugues
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Stephan Aßmus
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Stephan Aßmus
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Stephan Aßmus
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Stephan Aßmus
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Alexander von Gluck IV
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Jérôme Duval
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- John Scipione
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e - Axel Dörfler
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Brian Hague
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- looncraz
- » [haiku-development] Re: Final Set*UIColor Patch, Version 3e- Dario Casalinuovo