r23229 - Switch skins without restart => new FSkin API

[drdev]

It took longer than I had hoped, but I finally finished the major refactoring of skin support I've been working on that allows changing skins without restarting. It wasn't easy, but I think it makes the Theme/Skin support for Forge much nicer to use, and will hopefully encourage more skins to be created.

See below for details on the new implementation of this logic.

All developers should be aware that the way to use the FSkin class has changed. There are still getColor(), getFont(), getImage, and getIcon() functions for accessing skin-based properties, but they now return objects of type SkinColor, SkinFont, SkinImage, and SkinIcon respectively. I also added a getCursor() function and corresponding SkinCursor class for custom cursors. These Skin* objects should then be passed to a function with more or less the same name as the one you would have normally called, but rather than being on the component, it will be on an instance of ComponentSkin<T extends Component>, which is another new class that wraps all Skin* properties set for a given component such that they can be reapplied when switching skins.

To access the ComponentSkin object for a given component, simply call FSkin.get(comp). ComponentSkin has several subclasses (each with the name "<component class>Skin"), each with functions unique to a given component and any components extending that component, but FSkin.get(comp) will automatically return an instance of the one that fits it best, giving you access to all the functions you might need.

Below is a full list of available functions and examples for setting skin-based properties on different types of components. Please note that although I use FSkin.get on each line, it should probably be cached in a variable if it needs to be used more than once to save the performance of multiple HashMap accesses. Similar with caching Skin* objects retrieved from their associated functions if the same object would be used multiple times.

Code: Select all

/*ComponentSkin<T extends Component>*/
FSkin.get(comp).setForeground(FSkin.getColor(FSkin.Colors.CLR_TEXT));
FSkin.get(comp).setBackground(FSkin.getColor(FSkin.Colors.CLR_THEME));
FSkin.get(comp).setFont(FSkin.getFont(12));
FSkin.get(comp).setCursor(FSkin.getCursor(FSkin.LayoutImages.IMG_CUR_L, 16, 16, "CUR_L"));
FSkin.get(comp).setGraphicsColor(g, FSkin.getColor(FSkin.Colors.CLR_THEME));
FSkin.get(comp).setGraphicsGradientPaint(g2d, x1, y1, FSkin.getColor(FSkin.Colors.CLR_THEME), x2, y2, FSkin.getColor(FSkin.Colors.CLR_THEME).stepColor(30));
FSkin.get(comp).drawImage(g, FSkin.getImage(FSkin.ColorlessManaImages.IMG_0, 16, 16), x1, y1);

/*JFrameSkin<T extends JFrame> extends ComponentSkin<T>*/
FSkin.get(comp).setIconImage(FSkin.getIcon(FSkin.InterfaceIcons.ICO_FAVICON));

/*JComponentSkin<T extends JComponent> extends ComponentSkin<T>*/
FSkin.get(comp).setLineBorder(FSkin.getColor(FSkin.Colors.CLR_BORDERS));
FSkin.get(comp).setMatteBorder(top, left, bottom, right, FSkin.getColor(FSkin.Colors.CLR_TEXT));

/*JLabelSkin<T extends JLabel> extends JComponentSkin<T>*/
FSkin.get(comp).setIcon(FSkin.getIcon(FSkin.InterfaceIcons.ICO_FLIPCARD));

/*AbstractButtonSkin<T extends AbstractButton> extends JComponentSkin<T>*/
FSkin.get(comp).setIcon(FSkin.getIcon(FSkin.ButtonImages.IMG_BTN_START_UP));
FSkin.get(comp).setPressedIcon(FSkin.getIcon(FSkin.ButtonImages.IMG_BTN_START_DOWN));
FSkin.get(comp).setRolloverIcon(FSkin.getIcon(FSkin.ButtonImages.IMG_BTN_START_OVER));

/*JTextComponentSkin<T extends JTextComponent> extends JComponentSkin<T>*/
FSkin.get(comp).setCaretColor(FSkin.getColor(FSkin.Colors.CLR_TEXT));

/*JTableSkin<T extends JTable> extends JComponentSkin<T>*/
FSkin.get(comp).setSelectionForeground(FSkin.getColor(FSkin.Colors.CLR_TEXT));
FSkin.get(comp).setSelectionBackground(FSkin.getColor(FSkin.Colors.CLR_INACTIVE));

/*FPanelSkin<T extends FPanel> extends JComponentSkin<T>*/
FSkin.get(comp).setForegroundImage(FSkin.getIcon(FSkin.Backgrounds.BG_MATCH));
FSkin.get(comp).setBackgroundTexture(FSkin.getIcon(FSkin.Backgrounds.BG_TEXTURE));


Note that for SkinColor, the stepColor and alphaColor functions are now on the SkinColor object itself, returning another appropriately modified SkinColor object. Also note that anything that takes a SkinImage can also take a SkinIcon, as SkinIcon extends SkinImage. This includes ComponentSkin.drawImage, JFrameSkin.setIconImage, JLabelSkin.setIcon, AbstractButtonSkin.setIcon, AbstractButtonSkin.setPressedIcon, AbstractButtonSkin.setRolloverIcon, FPanelSkin.setForegroundImage, and FPanelSkin.setBackgroundTexture.

I did my best to keep these changes as minimal as possible, and ultimately I'm confident I found the best way without requiring far more extensive rewriting of the GUI code. I hope adjusting to these API changes will go smoothly for everyone involved, and please let me know if you have any questions or concerns. Also, if you can in the next several days, please try to check that skins are applied correctly on all views both when first starting Forge and after changing the skin, just in case I missed any screens or views in my own testing.

Thanks.
-Dan

[moomarc]

Thanks for this great undertaking, drdev! This is awesome and definitely improves skin testing! Also, while I didn't change skins often before, I find myself far more likely to do a quick switch to get a different feel for a match (I'm finally using my Simpsons skin again ).

[spr]

I think we have to try and come up with a better way than keeping persistent references to every single UI component simply for the sake of fast theme switching. For example, the game log consists of a list of JTextArea components which are cleared every time a new game is started. Except now, the skinning HashMap keeps references to the old components and so the memory used by these will not be recovered by the Java garbage collector.

I was wondering whether we could use reflection instead - starting at the root frame recursively iterate through every single child component and apply the skin that way? Or could we re-run the initialization code without having to restart Forge?

Something Maxmtg mentioned which I had never even heard of - weak references. Having done some belated research - Understanding Weak References - I think the WeakHashMap may be the way to go.

Cheers,
Steve

[drdev]

I'm certainly open to trying other options. the WeakHashMap in particular sounds promising. That said, iterating through the component tree alone is not enough, since I'm certain some components are not always present there yet we keep them around intentionally for later. Not to mention that iterating the component tree is at least as expensive as iterating the HashMap.

Feel free to try out other options yourself. I'm not sure what I'll have time for this week anyway.

Thanks.
-Dan

[drdev]

I just committed a large scale refactoring of the skinning logic in r24195. This change was needed after I realized that I wasn't able to really dispose of cached components reliably, and in some cases (such as the recent Dock Button bug I fixed), I was disposing of components that were removed from view but kept for reuse later.

Now, instead of components being cached in a dictionary mapping them to ComponentSkin objects, each of which also has a pointer to the Component, ComponentSkin objects will now be contained directly on the component object. This means no more FSkin.get(comp) calls or caching ComponentSkin objects in certain places. To set a components background color to a skin color, instead of FSkin.get(comp).setBackground(skinColor), you'll just call comp.setBackground(skinColor), making the code much cleaner.

To accomplish this, FSkin now contains a Skinned* class that extends each J* component used in Forge that requires skinning (such as SkinnedLabel extends JLabel). These classes each implement the ISkinnedComponent interface, which exposes a getSkin() function to return the ComponentSkin field of the component. They also defined set* functions for each skin property that can be applied to the component (such as setBackground, setForeground, setFont, setBorder, etc.), and override paintComponent to check if the Theme has been changed by the user and reapply all skin properties to the component if so. This way, when you change theme from the menu, it will automatically update everything currently visible, with other components updating the very next time they're painted.

Note that you shouldn't need to worry about the getSkin() function except in cases where you want to get a SkinColor property that's been set (such as comp.getSkin().getBackground() to return the SkinColor assigned as the background color). All set functions on ComponentSkin classes have been changed to protected, instead excepting you to call the function directly on the component.

Please keep in mind to use Skinned* instead of J* for components going forward if you need to be able to skin them. Note that all the F* classes have already been refactored to extend the corresponding Skinned* class (such as FLabel extends Skinned Label), so if you're using them you won't need to do anything special.

Hopefully this change will truly fix the memory leak issues resulting from the previous skinning implementation, in addition to making our code cleaner and easier to write for skinning logic. Let me know if you have any questions.

Thanks.
-Dan

[moomarc]

Thanks Dan. I've started implementing the new constructed match setup screen and have been using the F* components. Would it be better to switch to the Skinned* components instead?

Then one thing I noticed while testing my layout (although it's quite possible fixed with these changes), after a skin switch, combo boxes wouldn't use the new skin font colour until a restart happened. I'll test sometime this weekend and let you know if it works now.

[drdev]

Thanks Dan. I've started implementing the new constructed match setup screen and have been using the F* components. Would it be better to switch to the Skinned* components instead?

Then one thing I noticed while testing my layout (although it's quite possible fixed with these changes), after a skin switch, combo boxes wouldn't use the new skin font colour until a restart happened. I'll test sometime this weekend and let you know if it works now.

You should use F* classes where possible for a consistent look and feel unless you want to make something custom.

As for combo boxes, you need to use FComboBoxWrapper in order for them to update with a skin change.