The problem¶

By its nature and goal, the FWD GUI Themes shall be an independent drawing primitives library. All the data required for drawing should be passed into theme methods as Java primitive simple objects like integers, points, rectangles or strings.

This approach makes it possible using the Themes both for legacy 4gl widgets and any other widgets like widgets created as OCX replacement.

Meanwhile, in the current API there are multiple cases when widgets of particular complex 4gl types are passed to the theme methods.

Inside these methods, usually only a few widget object field values are extracted for drawing.

So these APIs must be changed, so these field values are always passed explicitly as method arguments.

The refactoring required is quite formal, and may be performed on method-by-method basis, so I do not see any risks to break things here.

As an example consider this (alphabetically first) method in Theme API:

public void drawCaptionButton(CaptionButton button,
                              GuiDriver gd,
                              int x,
                              int y,
                              int width,
                              int height)

Here the CaptionButton object is passed, just for its 4 attributes.

The replacement may look like this:

public void drawCaptionButton(GuiDriver gd,
                              CaptionButtonType type,
                              boolean enabled,
                              boolean pressed,
                              boolean small,
                              int x,
                              int y,
                              int width,
                              int height)

Here the button type enum, and three button features: enabled, pressed and small are passed explicitly. Now any program can use this method for any purpose.

The following code may be used to obtain the values from the widget.

boolean enabled = button.isEnabled();
boolean pressed = button.isPressed() && button.isMouseOver();
CaptionButtonType type = button.getType();
boolean small = false;
WidgetConfig cfg = button.topLevelWindow().config();
if (cfg instanceof WindowConfig)
{
   small = ((WindowConfig) cfg).smallTitle;
}

In practice, the code might be even simpler, since the caller usually knows the values to pass.

Also, some more boolean arguments need to be passed to this method for themes other than classic.

For the discussion see the posts #3820-691 to #3820-706.

As the implementation example, the https://proj.goldencode.com/attachments/download/9236/ClassicTheme.java file is attached with both old and new implementations (other stuff removed for simplicity).

All changes for this task will go to the 4231b branch.

The plan¶

1. Extract the list of API methods that need refactoring;
2. For each old method for each theme variant extract a new method matching the new interface, the old method now just extracts the values from the complex widget object and passes them to the new method. This work may be partly automated with the Eclipse "Extract method" refactoring function;
3. Test the new configuration (I may need help here);
4. Inline all calls to the "old" API. This might be done globally or on the call by call basis. The Eclipse "Inline method" refactoring function does it nicely;
5. Remove the "old" API.

Below is the first draft of the list of candidates for API change. Note: some of these calls, like Browser-related, are probably not worth changing, since these calls are probably will not ever used for anything but the 4gl browser itself:

1. draw3DBorder(ThreeDBorderGuiImpl border, GuiDriver gd, int x, int y, int width, int height)
2. drawBrowseColumnDrag(BrowseGuiImpl browse, GuiDriver gd, int x, int y, int width, int height)
3. drawBrowseColumnLabel(BrowseColumnGuiImpl column, GuiDriver gd, int x, int y, int width, int height)
4. drawBrowseColumnSetBackground(BrowseGuiImpl browseGui, GuiDriver gd, GuiColorResolver gc, int x, int y, int width, int height, boolean locked)
5. drawBrowseMarkersColumn(BrowseColumnGuiImpl column, GuiDriver gd, int x, int y, int width, int height)
6. drawBrowseNormalColumn(BrowseColumnGuiImpl column, GuiDriver gd, int x, int y, int width, int height)
7. drawBrowseResizingColumn(BrowseGuiImpl browse, GuiDriver gd, int left, int top, int right, int bottom)
8. drawBrowseRowHighlight(BrowseGuiImpl browse, GuiDriver gd, int x, int y, int width, int height)
9. drawBrowseTitlePanel(BrowseGuiImpl browse, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height)
10. (DONE) drawButton(ButtonGuiImpl button, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height)
11. drawCaptionButton(CaptionButton button, GuiDriver gd, int x, int y, int width, int height)
12. drawComboBoxButton(ComboBoxGuiImpl combo, GuiDriver gd, int x, int y, int width, int height, boolean pressed, boolean enabled, boolean hover)
13. drawComboBoxEntryField(ComboBoxGuiImpl combo, GuiDriver gd, GuiColorResolver gc, int x, int y, int width, int height, FontDetails font, boolean pressed, boolean enabled, boolean hover)
14. drawComboBoxFieldBorder(ComboBoxGuiImpl combo, GuiDriver gd, int x, int y, int width, int height, boolean pressed, boolean enabled, boolean hover)
15. drawEditorCaret(EditorGuiImpl editor, GuiDriver gd, int x1, int y1, int x2, int y2)
16. drawEditorContent(EditorGuiImpl editor, GuiDriver gd, GuiFontResolver gf, GuiColorResolver gc, int x, int y, int width, int height, NativePoint selStart, NativePoint selEnd, int dx, int dy)
17. drawFillInCaret(FillInGuiImpl fillIn, GuiDriver gd, int x1, int y1, int x2, int y2)
18. drawFrameTitle(FrameGuiImpl frame, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height)
19. drawLabel(LabelGuiImpl label, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height, boolean emptyMode)
20. drawLineBorder(LineBorderGuiImpl border, GuiDriver gd, GuiColorResolver gc, int x, int y, int width, int height)
21. drawMenuBarElement(MenuElement element, GuiDriver gd, GuiFontResolver gf, int x, int y, int width, int height, boolean hover) ?
22. drawMenuItemBackground(AbstractWidget item, GuiDriver gd, int x, int y, int width, int height, boolean hover) ?
23. drawMenuItemContent(AbstractWidget item, GuiDriver gd, GuiFontResolver gf, int x, int y, int width, int height, int textHeight, boolean focused) ?
24. drawProgressBar(ProgressBarGui pb, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height)
25. drawRadioButton(RadioButtonGuiImpl rb, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height, boolean focused, boolean hover)
26. drawRectangle(RectangleGuiImpl rect, GuiDriver gd, GuiColorResolver gc, int x, int y, int width, int height)
27. drawSelectionListItem(ComboBoxGuiImpl box, GuiDriver gd, GuiColorResolver gc, int x, int y, int width, int height, String text, int fontSize, boolean enabled, boolean highlighted, boolean selected)
28. drawSliderBackground(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, int x, int y, int w, int h)
29. drawSliderFrame(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, int x, int y, int w, int h)
30. drawSliderThumb(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, int x, int y, int w, int h)
31. drawSliderTicMarks(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, int x, int y, int length)
32. drawSliderTrack(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, int x, int y, int w)
33. drawSliderValue(SliderGuiImpl slider, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int w, int h)
34. drawTabBarWidgetItem(TabSetGuiImpl tabSetGui, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int width, int height, FontDetails customFont, String text, boolean selected)
35. drawTabBarWidget(TabSetGuiImpl tabSetGui, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, NativePoint p, NativeDimension d, ArrayList<TabSetItem> items)
36. drawTabPageWidgetItem(TabSetGuiImpl tabSetGui, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int width, int height, FontDetails customFont, String text, boolean selected)
37. drawTabPageWidget(TabSetGuiImpl tabSetGui, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, NativePoint p, NativeDimension d, ArrayList<TabSetItem> items)
38. drawText(TextGuiImpl textGui, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height)
39. drawToggleBox(ToggleBoxGuiImpl checkbox, GuiDriver gd, GuiColorResolver gc, GuiFontResolver gf, int x, int y, int width, int height, boolean pressed, boolean hover)
40. getBrowseCellForegroundColor(BrowseColumnGuiImpl column, boolean label, boolean selected, int viewportRowIndex) => ColorRgb
41. getBrowseCellSeparatorColor(BrowseGuiImpl browse) => ColorRgb
42. getColumnBackgroundColor(BrowseColumnGuiImpl column) => ColorRgb
43. getColumnLabelBackgroundColor(BrowseColumnGuiImpl column) => ColorRgb
44. getMenuItemHeight(MenuElement menuItem) => int
45. getRowToggleOffset(BrowseColumnGuiImpl column, boolean editor) => NativePoint
46. getRowToggleSize(BrowseColumnGuiImpl column, boolean editor) => NativeDimension
47. getSliderThumbImageName(SliderGuiImpl slider, boolean horiz) => String
48. getSliderThumbRegularDimension(SliderGuiImpl.TicMarks tm) => NativeDimension
49. getSliderThumbSmallDimension(SliderGuiImpl.TicMarks tm) => NativeDimension
50. setDownArrowImages(ButtonGuiImpl button)
51. setLeftArrowImages(ButtonGuiImpl button)
52. setRightArrowImages(ButtonGuiImpl button)
53. setUpArrowImages(ButtonGuiImpl button)

The drawButton() method signature has been changed: a ButtonGuiImpl instance is not required anymore for this method to work.
The changes were committed as 4231b, rev. 11532.

As this part starts its changes, does it make sense to use container classes to gather common parameters for these methods inside a one name space. It can simplify reading of the code.

Sergey Ivanovskiy wrote:

As this part starts its changes, does it make sense to use container classes to gather common parameters for these methods inside a one name space. It can simplify reading of the code.

I do not think so. The theme method is called exactly once in FWD. Creating a structure with the only purpose of making one call or two to a single function, adding code for packing/unpacking the structure in all themes would make things much less clear.

Besides, the user would need to switch his view every time he needs to see the documentation for a call parameter instead of just reading the method javadocs.

The drawComboBoxButton() method signature has been changed: a ComboBoxGuiImpl instance is not required anymore for this method to work.
The changes were committed as 4231b, rev. 11538.

Vladimir Tsichevski wrote:

The drawComboBoxButton() method signature has been changed: a ComboBoxGuiImpl instance is not required anymore for this method to work.
The changes were committed as 4231b, rev. 11538.

Just noticed that unless the mouse is hovering over the button or the button is pressed, the drawComboBoxButton() method does not paint the background. Is this behaviour intended?

Vladimir Tsichevski wrote:

Vladimir Tsichevski wrote:

The drawComboBoxButton() method signature has been changed: a ComboBoxGuiImpl instance is not required anymore for this method to work.
The changes were committed as 4231b, rev. 11538.

Just noticed that unless the mouse is hovering over the button or the button is pressed, the drawComboBoxButton() method does not paint the background. Is this behaviour intended?

This can be seen at least in Windows8Theme, I think, other themes do the same. I suppose it is a bug, since the button default background should match the theme and the background in pressed and hovered states, in my opinion.

Just noticed that unless the mouse is hovering over the button or the button is pressed, the drawComboBoxButton() method does not paint the background. Is this behaviour intended?

This can be seen at least in Windows8Theme, I think, other themes do the same. I suppose it is a bug, since the button default background should match the theme and the background in pressed and hovered states, in my opinion.

I agree this seems like a bug.

Ovidiu/Eugenie: What are your opinions?

Greg Shah wrote:

Just noticed that unless the mouse is hovering over the button or the button is pressed, the drawComboBoxButton() method does not paint the background. Is this behaviour intended?

This can be seen at least in Windows8Theme, I think, other themes do the same. I suppose it is a bug, since the button default background should match the theme and the background in pressed and hovered states, in my opinion.

I agree this seems like a bug.

Ovidiu/Eugenie: What are your opinions?

My point is the everything that is theme specific should be painted in particular specific theme. The theme independent parts can be painted within base class(ClassicTheme?). Everything that works not with this concept can be considered as a bug I guess.

Not sure I understand the issue. An screen-shot or two (on with current capture and one altered to reflect with how it should have been) would be helpful.

If the problem is that the combo is not sensitive to mouse hovers, then probably the event is not caught by the ComboBoxGuiImpl or its editor (I read that the button does act on this event).

If the background is of wrong colour (win classic grey), then probably the theme method is not implemented for Win10 so the super method is invoked instead. Since the Theme-s are independent, the ComboBoxGuiImpl and all other Impl classes should not impose a specific colour for any of the graphic element. If the ABL programmer selects a specific colour for an element, the Theme will find it in the palette it receives as usual parameter (GuiColorResolver gc) for all drawing methods.

Ovidiu Maxiniuc wrote:

Not sure I understand the issue. An screen-shot or two (on with current capture and one altered to reflect with how it should have been) would be helpful.

1. I did not change the behaviour. Moreover, I do not know how to change it since I do not know which background colors should be used in the themes.
2. I see an issue in that if some situations (mouse hovered, pressed) some background is painted by the call, and in other situations (normal button state), no background is painted, so the user is responsible for doing this, otherwise, the button is "transparent". Thus, the user should know at least which background color should be used for the current theme, which makes themes somawhat useless.

If the problem is that the combo is not sensitive to mouse hovers, then probably the event is not caught by the ComboBoxGuiImpl or its editor (I read that the button does act on this event).

1. The main goal of this task is to get rid of references to ComboBoxGuiImpl or any other concrete widget, so the ComboBoxGuiImpl is not related to the issue under discussion.
2. No, there is no problem with any mouse event.

If the background is of wrong colour (win classic grey), then probably the theme method is not implemented for Win10 so the super method is invoked instead. Since the Theme-s are independent, the ComboBoxGuiImpl and all other Impl classes should not impose a specific colour for any of the graphic element. If the ABL programmer selects a specific colour for an element, the Theme will find it in the palette it receives as usual parameter (GuiColorResolver gc) for all drawing methods.

1. No, the problem is that no color is really used since the background is not painted by the theme, so she Impl class has to "impose a specific colour" and paint background in some specific states.
2. The GuiColorResolver gc is not passed to this very call, so the theme always selects its own fixed color values. For example, here is the excerpts from the method implementation for Windows8Theme:

      if (enabled && isDropDown && (mouseOver || pressed))
      {
         gd.setColor(pressed ? new Color(0xCCE4F7) :
                     /*hover*/ new Color(0xE5F1FB));
         gd.fillRect(x + width - COMBOBOX_BUTTON_WIDTH, y, x + width - 1, y + height - 1);
      }

As you may see:

1. The background is painted in mouseOver or pressed situation only, and is not painted otherwise;
2. Some fixed hexadecimal numbers used as the fixed color values, no values could be provided by the user.

Ovidiu Maxiniuc wrote:

The button background is transparent and the parent frame (part of the combo widget) is responsible for drawing the entire area.

Yes, for ComboBoxGuiImpl the call to drawComboBoxEntryField() does two things:

1. Calculates the background color based on input flags and paints the background for the whole combo-box are, including the button area;
2. draws the entry field, as the method name suggests.

Where the code for the first part is:

      if (!enabled)
      {
         gd.setColor(isDDL ? new Color(0xFFFFFF) :
                             new Color(0xCCCCCC));
      }
      else if (isDropDown)
      {
         gd.setColor(new Color(0xFFFFFF));
      }
      else
      {
         // SIMPLE & DROP_DOWN_LIST cases
         gd.setColor(pressed ? new Color(0xC6E7F7) :
                       hover ? new Color(0xBDE7FF) :
                    /*normal*/ new Color(0xFFFFFF));
      }
      gd.fillRect(x, y, width, height);

I think, this color calculation shall be extracted into its own call, for example, Color getComboBoxColor(enabled, isDropdown, isPresses, mouseOver), so it can be used later in both drawComboBoxButton() and drawComboBoxEntryField() methods.

Task branch 4231b has been merged to trunk as revision 11347.