Community
Participate
Working Groups
We have instances of system colors that are being referenced in tons of paintable objects. Now that we get notification when a system color changes, it would be nice to be able to update the instance of the Color that referred to it. This way there is no need to notify tons of objects that are holding onto the old Color to pick up the new "constant". An alternative approach is to create a subclass of Color called SystemColor which is automatically reinitialized whenever the system color changes. Or maybe a more generic MutableColor subclass. Of course, a color's changing after it has been set on a widget would have no affect. This behavior has precedence in Images that are set on buttons or toolitems (I can't remember which) on GTK. If you update the Image, the widget is still displays an old copy of the Image.
I had considered this but eventually rejected the idea. >We have instances of system colors that are being referenced in tons >of paintable objects. Why not query the color from display when needed? In that case, there would be no need to listen for setting changes and your paintable objects will always be right.
(In reply to comment #1) > Why not query the color from display when needed? In that case, there would That doesn't help for the same reason that invoking canvas.setForeground (Display.getCurrent().getSystemColor(SWT.COLOR_INFO_FOREGROUND)) would not automatically update the foregournd Color later on. What were your reasons for rejecting this idea, Steve? Even if you can't reinit the Color, SystemColor could use the adapter pattern and act as a wrapper that points to the up-to-date Color. We can't do this ourselves since Color is final.
Color objects (and other graphics objects for that matter) are not unique. If you set a color into a control and then query it back, depending on the control and the platform, you might get then identical color back or an equal () color. This means that there can be copies of system colors. >>That doesn't help for the same reason ... If you never set a color, the default color is used and will change automatically when the theme changes. I realize that you may have a framework in place that prevents you from knowing whether a color should be set or not.
No one has said anything about asking to get the color back from a GC or Widget. If you return a different instance and even a different color, it doesn't affect our scenario. Unfortunately, our system color constants are declared on an interface, so they are final and can't be updated even with the current notification. We would have to break API and make it a class. Treating system colors specially might be safer than just reinitializing any color. on win32, it might look like this: class GC { ... public void setForeground(Color color){ ... setPen(color.handle(), -1,-1,-1,-1,data.dashes); } ... } class SystemColor extends Color { ... int handle() { return OS.GetSysColor(sysID); } ... } Of course I have no idea how slow that native method is so it might be a stupid idea/non-portable
> Why not query the color from display when needed? In that case, there would > be no need to listen for setting changes and your paintable objects will > always be right. If you have a generic paintable object like a Polygon, etc., there is no easy way to set its background to be "query the display for the system's xxx color", right? I mean, its color can be red, or it could be widget_background, both are just stored in a field typed as Color. For us to do it your way we would have to wrapper SWT's color and create the SystemColor concept ourselves. Or we could create two fields to store either a color or system "int" identifier.
The code you gave won't work in general. If you set a SystemColor in a widget and then query it back, on some platforms, you would get a Color and this would no longer be correct. If you have a field in the polygon that represents the color, don't initialize it and always use an accessor. Inside the accessor, if the color is null, get the system color. If the color is not null, return it. If someone sets the color, they are no longer using the system color. I have no idea whether this would work for you but this is the approach that custom widget writers have been using for a while. It's not great, but it works.
So, a system color might work for us but not for widgets, which don't hold onto the java instance. I think the showstopper would be if the widget didn't track the system color. I think the original request is the most reasonable one and it doesn't introduce anything too complicated like a mapping of system constants->(widget+ fg/bg).
Not sure what you mean. The original request doesn't work.
Why does the original request (the summary) not work? Colors would be exactly like Images. If you modify an Image by painting on it, you are only modifying that image. If you set the Image on a Button and get it back, it could be a different java object and a different resource. Painting on the image after setting it to a widget may have no effect. It's the same, no?
>If you modify an Image by painting on it, you are only modifying that image. Depends on the platform and the control. For example, custom control that don't take a copy of the image will be updated. On Windows, even the native controls are different. Trees and tables take a copy, menu items don't etc. Hammering a single instance and hoping that no one has any copies seems wrong to me. It's not what the operating systems do and it doesn't work in general (although it might work for GEF). Sorry but I'm going to have to WONTFIX this bug report.
> Hammering a single instance and hoping that no one has any copies seems wrong > to me. OK, but HOW is it different than the Image scenario? It's exactly the same, except it's even more consistent whereas the Image scenario varies on each platform.
Nope. Both are the same. If you set a color into some native controls and then get it back, you might get back a copy that won't get hammered.
A color is a cross-platform reference to some OS resource. Reinitializing is just changing that reference. SWT clients already know that widget.setColor() is just a cross-platform way of handing over the current resource, and not the java object itself. They must be aware of this for the other reasons you mentioned. This is not GEF-specific. It helps anyone that works with SWT's colors without reaching in and grabbing the platform-specific data, which is everyone except for widgets. For example, JFace's ColorProvider and ColorDecorator classes. Also syntax highlighters, and even StyleRange used by StyledText control.
(In reply to comment #12) > Nope. Both are the same. That's what I said. It's the same. So let clients change a Color just like they can currently create a GC on an Image and change the Image. Or as you call it, hammering an _Image_ and hoping that no one has any copies.
This is the last time I'll try. Colors are not unique. There can be 1,000,000 instances of color, each one having the same operating system handle. I don't have a refererence to every color that is creates so I can't find all the instances and hammer the handle.
That is not what we are asking you to do and it wouldn't even be the correct behavior. Some manually-created colors might happen to have the same handle as a system color and it wouldn't make any sense to update them. The scenario is for the non-widget world that must hold on to specific instances of colors, regardless of whether they have the same handles or not.
I have no idea what you are talking about. Please submit a patch.
I'm not sure I understood your use of "hammer" the first time. I thought you were referring to the client making the change to the color. If you meant SWT updating the handle for a single color instance, this is all that we are asking for. It wouldn't make sense to update copies of the color. Here's the patch to Color.java: /** * Disposes the current color reference and reinitializes this * color using the given values. Equivalent to calling: * <pre>dispose(); * new Color (device, r, g, b);</pre> */ public void disposeAndInit(Device device, int r, int g, int b) { dispose(); init(device, r, g, b); } /** * Used to update a Color that was created using Display#getSystemColor. */ public void initSystemColor(Device device, int id) { // copy code from Display or refactor it. } The first method is exactly the same as calling dispose() on the Color, and then creating a new color. The effect on the other 1,000,000 "copies" is the same in either case. The second method is exactly the same as just calling Display.getSystemColor (int) and garbage collecting the old instance (since disposing isn't necessary). The only difference in both cases is that the java object is reused.
>If you meant SWT updating the handle for a single color instance, this is >all that we are asking for. Ok, and I have already described how this does not work in general (while it may work for your use of system colors). Updating a single instance is a lie. Anyone who sets and then queries a color back will be burned. The new instance won't be updated and they will have no idea why their code fails. WONTFIXTHISEVER.
> Ok, and I have already described how this does not work in general (while it > may work for your use of system colors). Updating a single instance is a > lie. But it's exactly what the proposed API would state, so how is it a lie? > Anyone who sets and then queries a color back will be burned. The new > instance won't be updated and they will have no idea why their code fails. Using your exact same scenario, someone who sets and then queries a color back, and then disposes the *original* instance, will be "burned" because now they have a color with a bogus handle (on 8-bit displays). What is your solution to this scenario? There is no solution because it's incorrect usage and it's a pointless example.
Actually, having copies of the Color isn't even necessary. There could be references to the handle in the native widgets. I could just set the color on a TreeItem and the dispose the color. SWT does not go in and repair the reference to the handle of the disposed color. If I set a font on a control and then dispose the Font SWT doesn't go and repair references to that handle either. Or, what if I have a copy of a Font and I dispose the original? Does SWT go in and hammer the copies to indicate that the copies are also disposed? No. Try the following: Font font = new Font(display, "Papyrus", 18, SWT.BOLD); shell.setFont(font); Font another = shell.getFont(); font.dispose(); System.out.println(another.isDisposed()); Any copies of the Font are completely invalid, just like colors.
I have been trying to follow this conversation but I find it quite confusing. Randy, can you please post a patch (like a real one, team > create patch) that shows what you are proposing? I find your notes from comment #18 too vague.
(In reply to comment #22) > I have been trying to follow this conversation but I find it quite confusing. > Randy, can you please post a patch (like a real one, team > create patch) that > shows what you are proposing? I find your notes from comment #18 too vague. Any patch I would submit would not involve updating copies of colors. So the first step is to convince Steve that NOT updating copies is correct and consistent with the way dispose() works for both Fonts and Colors.
The other scenarios involving disposing fonts and colors are programming errors. You should not dispose a resource that you did not allocate.
(In reply to comment #24) > The other scenarios involving disposing fonts and colors are programming > errors. You should not dispose a resource that you did not allocate. If you look at the code, I am disposing the resource that I created.
Your argument went something like "It's wrong for fonts that get disposed, so it's ok to get it partially right for system colors".
The argument is a color is a useless java object once its been disposed. So let me reuse that object to do something else.
Eh? Once an object is disposed, you can't use it (or reuse it) for anything.
disposed != GCed
Once an SWT object with associated operating system resources is disposed, it cannot be used (reused) for anything. The garbage collector doesn't enter into it.
So this is the real issue. How important is it that such resource pointers be "immutable"? What does it guarantee?
Steve, please reconsider this. We have system colors declared on an interface, which means there is no other way we can update them. Even if we deprecate the interface and create a class to replace it, it still wouldn't help us.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it and remove the stalebug whiteboard tag. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. --
