GNUstep themes [Was: Obscure timing issue]

[Fred Kiefer]

I may be totally misunderstanding this whole issue, but to me your mail
sounds as the only potential blocking argument against my proposal is
that the mechanism to select a new theme may need to be a bit more
complex then now. Lets look into that later.


Richard Frith-Macdonald wrote:
> On 10 Dec 2008, at 23:10, Fred Kiefer wrote:
>> I don't quite understand this argument. Currently we have the same
>> complexity (two implementations for the same thing) mixed into one
>> class. To me separating them into two classes sounds like a reduction in
>> complexity.
> 
> I'm primarily thinking of complexity for the user/designer of themes ...
> if you have two different theme classes with different capabilities,
> then they need to pick one of them to use, and re-implement facilities
> they want from the other one (or limit what they can do) ... which is a
> real pain if they are not a programmer (and a waste of their time if
> they do know how to program)!  Separation into two classes cannot reduce
> complexity in fact ... just duplicate code.
> Or ... perhaps what you really mean is two options ... one 'theme' which
> is the current hard-coded look and would never be used for developing
> themes, and another theme which does essentially what the current theme
> does and people can use for new themes, but what would be the point of
> doing that?

To me the GNUstep theming code currently serves two purposes. It defines
the API a custom theme needs to implement and it also provides a way to
implement a theme without writing code, by just selecting some images.
Both of these are important. What ever ways we will offer for themes,
people will come up with differing requests and tiles may not be enough
for them. Just think of what Camealon does or a potential Windows theme.
What I am suggesting is to separate these two concepts also in two classes.
What I don't see is how this would duplicate code, as these classes will
inherit from another and that way share most of the code.

>> Looking into GSTheme.m I have the impression that half the code there is
>> only used when using tiled themes. Understanding this class and its
>> concepts could get easier by moving this code out. And with out I mean
>> into a separate file not removing it from gui altogether. (although
>> putting it into a separate bundle might be an option)
> 
> Well that's a different matter and has nothing to do with producing
> separate theme classes ... I would have said that the current theme code
> is small/simple enough to be easy to understand still, but nearing the
> point where separation for organisational purposes could be useful ...
> but that's simply separation of the class into multiple files with
> different categories in each.  The obvious break down would be:
> 1. the code dealing with loading and dynamically changing themes (this
> is the core)
> 2. the code providing helper methods (such as tiling)
> 3. the code providing gui element drawing. At the moment this really
> only implements buttons, but it's where most of the theme coding needs
> to be done, and in the long run you might even want a separate files for
> the drawing methods for each major gui element.
> Now GSTheme is already broken down in precisely this manner, just not
> separated out into individual files at present.
> 
As you write, we don't have much methods in that class that already
support full theming. Lets look at one of those:
- (void) drawButton: (NSRect)frame
                 in: (NSCell*)cell
               view: (NSView*)view
              style: (int)style
              state: (GSThemeControlState)state
{
  GSDrawTiles   *tiles = nil;
  NSColor       *color = nil;

  if (state == GSThemeNormalState)
    {
      tiles = [self tilesNamed: @"NSButtonNormal" cache: YES];
      color = [NSColor controlBackgroundColor];
    }
  else if (state == GSThemeHighlightedState)
    {
      tiles = [self tilesNamed: @"NSButtonHighlighted" cache: YES];
      color = [NSColor selectedControlColor];
    }
  else if (state == GSThemeSelectedState)
    {
      tiles = [self tilesNamed: @"NSButtonPushed" cache: YES];
      color = [NSColor selectedControlColor];
    }

  if (tiles == nil)
    {
      switch (style)
        {
          case NSRoundRectBezelStyle:
          case NSTexturedRoundBezelStyle:
          case NSRoundedBezelStyle:
            [self drawRoundBezel: frame withColor: color];
            break;
          case NSTexturedSquareBezelStyle:
            frame = NSInsetRect(frame, 0, 1);
          case NSSmallSquareBezelStyle:
          case NSRegularSquareBezelStyle:
          case NSShadowlessSquareBezelStyle:
            [color set];
            NSRectFill(frame);
            [[NSColor controlShadowColor] set];
            NSFrameRectWithWidth(frame, 1);
            break;
          case NSThickSquareBezelStyle:
            [color set];
            NSRectFill(frame);
            [[NSColor controlShadowColor] set];
            NSFrameRectWithWidth(frame, 1.5);
            break;
          case NSThickerSquareBezelStyle:
            [color set];
            NSRectFill(frame);
            [[NSColor controlShadowColor] set];
            NSFrameRectWithWidth(frame, 2);
            break;
          case NSCircularBezelStyle:
            frame = NSInsetRect(frame, 3, 3);
          case NSHelpButtonBezelStyle:
            [self drawCircularBezel: frame withColor: color];
            break;
          case NSDisclosureBezelStyle:
          case NSRoundedDisclosureBezelStyle:
          case NSRecessedBezelStyle:
            // FIXME
            break;
          default:
            [color set];
            NSRectFill(frame);

            if (state == GSThemeNormalState || state == GSThemeHighlightedState)
              {
                [self drawButton: frame withClip: NSZeroRect];
              }
            else if (state == GSThemeSelectedState)
              {
                [self drawGrayBezel: frame withClip: NSZeroRect];
              }
        }
    }
  else
    {
      /* Use tiles to draw button border with central part filled with color
       */
      [self fillRect: frame
           withTiles: tiles
          background: color
           fillStyle: GSThemeFillStyleNone];
    }
}

Half the code does the old drawing, the other half is about tiles. They
have nothing in common and the method would surely be easier to read,
when we separated them out. Which is exactly what I propose.

>> As soon as you switch your theme setting to TiledTheme.theme (or
>> whatever we call it) you have similar code as now, that will just fall
>> back to the non-tiled super implementation, when no tile is found.
> 
> That really does sound like you are advocating code duplication ... with
> a theme which can't be used for anything but drawing the traditional
> gnustep look, and another theme which follows the current design of
> drawing the traditional look but allowing people to use image tiling and
> other mechanisms to create new looks without (or with) any programming.
> As far as I can see that has three problems ... extra work to do it,
> code duplication, and a tendency to work against development of a real
> theme engine since a lot of people would just use the traditional
> look/feel and thus not test the theme engine at all.

As these implementation would inherit, the tile theme could just fall
back to the old drawing code, when there are no tiles.
As for the problems:
- extra work -> I am offering to do it
- code duplication -> I don't see it
- less testing of theme engine -> This is quite possible, but then
again, I don't think the current tiles theme gets used that much. We
really should rather focus on providing a clear interface for themes,
with that in place a full tiling theme will be as easy as any other theme.

>> I
>> don't understand, why this would force people to write code that don't
>> have to do it currently.
> 
> I thought you meant code separation rather than code duplication ... if
> things were separated then people would need to write code because
> features they needed were not in the theme they were trying to use.  If
> code ids duplicated, that particular issue goes away.

I really don't understand this paragraph. As I explained below, you
either inherit from the tile theme or the standard drawing theme and as
they inherit from each other I don't see how you could miss a feature.
Other then inheriting from the standard drawing theme, but requiring
tile and here a switch to the other super class resolves the issue.

>> Sorry, I can only see a big misunderstanding here. Once again the new
>> concept:
>> - One standard theme class that just has the current (or old) gui
>> drawing code for the different user interface elements.
>> - One non-standard tiled theme class, also delivered with gui (perhaps
>> as a bundle to demonstrate how to write theme bundles?). This will be a
>> subclass of the first and fall back to super calls when no tiles are
>> found.
>> - Multiple theme provided by other people either subclasses of the
>> standard or the non-standard theme. (OK the later might get a little
>> harder with a bundle)
> 
> Well,  I think I understand it better now ...
> 
>> - One standard theme class that just has the current (or old) gui
>> drawing code for the different user interface elements.
> 
> 
> I'm against this because such a class *as standard* would mean that
> theme code in the gui would be tested less well, and because it would be
> duplication of code, and because it would require work to do it.
> 
See above.

>> - One non-standard tiled theme class, also delivered with gui (perhaps
>> as a bundle to demonstrate how to write theme bundles?). This will be a
>> subclass of the first and fall back to super calls when no tiles are
>> found.
> 
> This is what the existing theme engine already provides ... but it's
> fundamentally not intended as a example of how to write theme bundles,
> rather it's intended as a mechanism for non-technical people to be able
> to design/develop themes *without having to write anything*!  This is
> the key concept of theming ... to put it in the hands of the designer
> rather than the programmer.

This sounds as if you are against any other themes that want to do more than
just provide tiles.
With my propose people would still have the same functionality to write
themes without any coding.

>> - Multiple theme provided by other people either subclasses of the
>> standard or the non-standard theme. (OK the later might get a little
>> harder with a bundle)
> 
> Again, this does not go far enough.  With the current design we allow
> for these things but we also allow for theme bundles with are *not*
> subclasses and do not contain code. Such bundles are pure data,
> consisting images and defaults settings etc controlling how the standard
> theme engine draws things (currently these are the only themes supported
> by Thematic.app, but it would be nice to add support for subclassed
> themes too).
> However, they depend upon the facilities in the theme engine ... so
> having a standard theme which did not support this would mean that we'd
> need extra code to automatically switch to the other packaged theme when
> necessary.

No this is the only real problem I see, the one I mentioned in the first
paragraph of this mail. And now is that time to talk about it.
Currently when a theme gets loaded that doesn't provide a principle
class we create an instance of GSTheme and return that (see
loadThemeNamed:). Wouldn't it be just as easy to create an instance of
GSTiledTheme and use this?

> So, while none of the problems introduced by having a cut-down standard
> theme would be huge, they do amount to a compelling reason not to do it,
> given that (fro the point of view of theming at least) it has no
> advantages to counter them.

I don't follow you here. But there is one other thing that really
surprises me in this debate: Nobody else seems to be interested in it.
Perhaps it is just the mail subject that keeps people away. Or themes
aren't that hot as the used to be?