Created attachment 1626 [details]
openbox with plugin support (First Draft)

In this archive, I separate frame and frame render into plugin. The path of plugin is currently hard coded and loaded at earliest as posible. The interface is weak and, imo, will be re-thought.

Created attachment 1628 [details]
patch

Created attachment 1629 [details]
New patch

Patch note :

 * Split configuration of frame from original theme.h.
 * Add support of per theme plugin load
 * Load module from HOME directory in .config/openbox/frameplugins/
 * Provide 3 plugin :  default, concept and minimal
   - minimal is 0 conf from default one
   - default is same as the default theme of openbox
   - concept is a minimal like nodecor, keepborder of openbox.
 * fall back to minimal plugin if no plugin is define.

Note: I can't compile openbox-gnome-control-center from this patch.

To use this patch : 

./configure
make

and link or copy frameplugins/*.la in $HOME/.config/openbox/frameplugins/
run openbox from openbox directory in source directory

edit themerc of theme and add this line :
frame.theme.engine: libconcept.la
to choose which plugin you want use.

Concept theme use this values :
concept.border_width: 4
concept.focus_border_color: #000000
concept.focus_corner_color: #ff0000
concept.unfocus_border_color: #0000ff
concept.unfocus_corner_color: #00ff00

Some isues still unknow, like how plugin are unloaded. You can give feedback.

Merry Christmas

Note : The last patch is from original source code of openbox.

Created attachment 1635 [details]
Update version that patch openbox 3.4.5

Same as last but merged with 3.4.5.

Created attachment 1651 [details]
Patch with plugin for themes suport

This patch remove all shared data in ObFrame,
And it propose an interface for this plugin,
My oppinion is that FramePlugin must be independant from ObClient, that is not currently the case, i.e. do not need an ObClient.
It's the first step in this direction.

Currently only libdefault.la engines should work, engines plugin are store in $HOME/.config/openbox/engines/

Best regards

Have you looked at openbox.git yet?  I think this stuff should be done against that rather than against 3.4 releases.  You should have a look. :)

I am wondering how come you are linking to gdk pixbuf?

I will look at the patch in detail soon.  In openbox.git, I am working on adding support for compositing.  You may definitely want to take that into account.  Maybe the compositor rendering should be a part of the same plugin, or maybe not.. I'm still pretty confused how to integrate the compositor nicely.

> Have you looked at openbox.git yet?  I think this stuff should be done against
> that rather than against 3.4 releases.  You should have a look. :)

I just look at git but I don't know how it's work currently (I'm a basic subversion user). I am not opposed to use it.

> I am wondering how come you are linking to gdk pixbuf?

This patch use gdk_pixbuf because I provide an early plugin which use it. It's very simple plugin which draw frame border from picture (PNG and gdk_pixbuf suported format). The default plugin (that make the same layout of current openbox) and openbox-core do not need it.

> I will look at the patch in detail soon.  In openbox.git, I am working on
> adding support for compositing.  You may definitely want to take that into
> account.  Maybe the compositor rendering should be a part of the same plugin,
> or maybe not.. I'm still pretty confused how to integrate the compositor
> nicely.

Do not hesitate to ask any question you think relevant.

The current plugin interface do not include Popup because I prefer to finalize the frame interface before. The main drawback of this cituation is that openbox store some theme configuration twice.

I do not know how the compositor work, but if it only need X11 windows handler, the plugin can be the right place to implement it.

Created attachment 1658 [details]
Patch folow master git.

I make this patch with git, from the master branch of git://git.icculus.org/mikachu/openbox

To use it you should put libdefault.la in $HOME/.config/openbox/engines . libdefault.la is the default plugin that openbox-core load if themerc don't specify it. This plugin do things as current theme, with some little bugs.

thanks for doing that!  i will put this in a public branch ("plugin") so you and others can see it and make further diffs against it easily

also i should say that i haven't really looked at the changes i earnest yet.  however i am going to be taking a 5 hour bus ride this weekend so I plan to look at them during that in great detail.  and then I will provide some feedback.

do you use IRC?  most of our dev thinking and coordinating happens on IRC and it would make it much more possible for us to collaborate if you were around there.

one thing is:  the indenting style you are using differs a lot from the openbox code, and i'm really very anal about code style.  so that is going to have to get fixed.

here's a quick idea from the HACKING file
Indentation
-----------
For openbox, we aim to have consistent coding style. Some, but surely
not all, guidelines:
 * use 4 space indents
 * tabs should not appear in source files
 * functions should have the opening and closing braces on their own
   lines
 * most other constructs should have braces on the same line as the
   statement
 * else appears on a new line, just like an if
 * when in doubt look at the rest of the source
 * vim users can use "set expandtab tabstop=4 shiftwidth=4
   softtabstop=4" for some of this

one last thing...

i've been working with render and added some nice things to it.

it now has an image type, and an image cache that caches various sizes of the same image.  in thinking about this and plugins, i think that maybe the whole of libobrender would become part of a plugin, or at least would be used by one or more plugins, and provide abstractions to openbox so openbox would not use any of libobrender directly.

what say you?

> do you use IRC?  most of our dev thinking and coordinating happens on IRC and
> it would make it much more possible for us to collaborate if you were around
> there.

I do not use IRC day-to-day, but I can use it.

> one thing is:  the indenting style you are using differs a lot from the openbox
> code, and i'm really very anal about code style.  so that is going to have to
> get fixed.

I use eclipse to develop. I will see how to setup eclipse to match to openbox code format.

> i've been working with render and added some nice things to it.
> 
> it now has an image type, and an image cache that caches various sizes of the
> same image.  in thinking about this and plugins, i think that maybe the whole
> of libobrender would become part of a plugin, or at least would be used by one
> or more plugins, and provide abstractions to openbox so openbox would not use
> any of libobrender directly.

I do not look at libobrender currently (that why I use gdk_pix_buff), but I agree that libobrender is used by plugins or is a part of plugins. Currently an example is libdefault that use libobrender.

Thanks for git branch :)

Best regards.

Created attachment 1662 [details]
path to remove tabs and make code more closer to openbox source layout

ok i have applied this.  thanks :)

since it basically changed everything, i merged it with the previous diff, as i think that will just be easier to read in this case. :)

Created attachment 1663 [details]
Incremental patch

Created attachment 1664 [details]
Incremental patch 0004

Created attachment 1665 [details]
Incremental patch 0005

Fix bug of frame title

Note : currently, I only maintain the default plugin. Others are supposed to not work.

Created attachment 1666 [details]
Improve interface by removing some stuff

Created attachment 1678 [details]
Remove the keep border option

Created attachment 1679 [details]
Keep border is a part of plugin option (no a global option)

hi I read through the first 2 diffs (I haven't looked at anything beyond those in my git tree yet), and it is a really great start!  Here were some of the thoughts I had about where to go with it.  I can help with this stuff, but it's all my thoughts so far on it:

some things i want in openbox, not the engines (some of this is important to
 allow reconfigure to change theme plugins):
  - creation of the frame top level window
  - map/unmap of top level and client windows (can have pre/post funcs for the
    engines) (engine should not deal with ignore_unmaps or grabserver)
  - reparenting of client into/out of the frame top level window
  - shape adjusting from the client (can merge with a shape provided by the
    engine)
  - flashing end-time should come from openbox
  - iconify animation should move to compmgr
  - adjusting the focus_cycle_indicator
  - wrapper around engine funcs to handle root/menus/dialogs/moveresize, and
    it calls plugin-specific one if it cant pick a context
  - frame<->client gravity translations
  - frame_rect_to_frame

80 columns

consistent use of "engine" vs "plugin".  prefer "engine" everywhere

combine set_client_area and update_layout

why is update_skin needed to be public? it should be called internally in updat\
e_layout and when flags change and stuff
the frame_adjust_area->update_layout conversion didn't preserve the same
  booleans in the call in multiple places, are they correct that way?
also moved boolean in update_layout should probably stay (or maybe
  moved/resized can both be implicit if we pass a rect and "fake"
  boolean only, by merging in set_client_area)
avoid using Rect as return type or fn parameter, use a Rect* in the arguments
  instead, avoids excessive copying of rects
consistant render_plugin->func() vs just func()
  - want an engine.c/h that has all the funcs, and wraps calls to the function
    pointers.  for some it will perform functionality (like for grabbing the
    client for example) and can call multiple fn pointers into the actual
    engine
don't make engines depend on X db for the config, actually we will probably
  pass an xml tree to the engine instead, have to dig up the old xmltheme code
  again and forward-port it (96bb892054b, fdc10b75d648, 69a5c9c2344), there are
  more than a few changes to the xdrb themes since then so this needs some
  care
  - have the theme choose the engine, so user chooses a theme, rather than
    having user choose engine and also theme
all variable definitions at the top of the block
why update_layout when focus changes?
make a create/destroy_event_window() function which everyone including
  engines can use that wrap XCreate/DestroyWindow and also calls
  add/remove_window for the event hashtable, pass fn pointer for this to engine
  if we need to. window_map should remain static
there is a bunch of random indenting changes in tests/*.c
what would the plugin1-4 triggers be used for, how would openbox know when to
  fire them?


i will look at the further patches when i can. :) thanks!

Hello, Thanks for your support and feedback.

(In reply to comment #24)
> hi I read through the first 2 diffs (I haven't looked at anything beyond those
> in my git tree yet), and it is a really great start!  Here were some of the
> thoughts I had about where to go with it.  I can help with this stuff, but it's
> all my thoughts so far on it:
> 
> some things i want in openbox, not the engines (some of this is important to
>  allow reconfigure to change theme plugins):
>   - creation of the frame top level window

It's plan.

>   - map/unmap of top level and client windows (can have pre/post funcs for the
>     engines) (engine should not deal with ignore_unmaps or grabserver)

It's plan.

>   - reparenting of client into/out of the frame top level window

It's plan.

>   - shape adjusting from the client (can merge with a shape provided by the
>     engine)
I don't know well shape, but it could be done ^^
>   - flashing end-time should come from openbox

It's plan.

>   - iconify animation should move to compmgr

Does it mean that I can remove animating structure ?

>   - adjusting the focus_cycle_indicator

It's plan.

>   - wrapper around engine funcs to handle root/menus/dialogs/moveresize, and
>     it calls plugin-specific one if it cant pick a context

It's plan, but I prefere to stabilize frame structure before.

>   - frame<->client gravity translations

I will see what does it mean in term of development.

>   - frame_rect_to_frame

Same as the last one ?

> 
> 80 columns
> 
> consistent use of "engine" vs "plugin".  prefer "engine" everywhere

It's plan ^^

> combine set_client_area and update_layout

It's plan.

> why is update_skin needed to be public?

For granularity issue, but currently I don't know why ^^

> it should be called internally in
> updat\
> e_layout and when flags change and stuff
> the frame_adjust_area->update_layout conversion didn't preserve the same
>   booleans in the call in multiple places, are they correct that way?

The moveresize boolean is disign to specify if the client window must be resized (if we draw a moveresize frame). In the current code the default plugin don't implement this behavious, as far as remember. The source is a draft, I don't change a lot of code of default theme engine. I adapt the code "lazyly".

Note : this behaviour will be deprecated since frame top level window and client window will be managed by client.

> also moved boolean in update_layout should probably stay (or maybe
>   moved/resized can both be implicit if we pass a rect and "fake"
>   boolean only, by merging in set_client_area)

I agree.

> avoid using Rect as return type or fn parameter, use a Rect* in the arguments
>   instead, avoids excessive copying of rects
> consistant render_plugin->func() vs just func()

This feature was done to avoid write access to the rect used by frame. It's can use Rect * with a copy of Rect from frame, it was done in conceptual point of view. Pragmatic view would be done with Rect* on frame.

>   - want an engine.c/h that has all the funcs, and wraps calls to the function
>     pointers.  for some it will perform functionality (like for grabbing the
>     client for example) and can call multiple fn pointers into the actual
>     engine

You want I generalize what I had done on func "plugin_frame_context" in engine_interface ?

> don't make engines depend on X db for the config, actually we will probably
>   pass an xml tree to the engine instead, have to dig up the old xmltheme code
>   again and forward-port it (96bb892054b, fdc10b75d648, 69a5c9c2344), there are
>   more than a few changes to the xdrb themes since then so this needs some
>   care

I'm agree that we can avoid the using of X db, but imo xml is not the best config file fomat (troll ?) but I can follow the guide line. We can provide a library to read config file (obparse ?)

>   - have the theme choose the engine, so user chooses a theme, rather than
>     having user choose engine and also theme
> all variable definitions at the top of the block

Currently the theme engine is selected by theme in themerc by "theme.engine" token and openbox fallback to libdefault.la if it is not set in themerc.

> why update_layout when focus changes?

I do not remember, safe development, I think :p

> make a create/destroy_event_window() function which everyone including
>   engines can use that wrap XCreate/DestroyWindow and also calls
>   add/remove_window for the event hashtable, pass fn pointer for this to engine
>   if we need to. window_map should remain static

It's plan ^^

> there is a bunch of random indenting changes in tests/*.c

I don't remember to edit any tests/*.c, can they be merged/replaced from master branch ?

> what would the plugin1-4 triggers be used for, how would openbox know when to
>   fire them?

They are provided to permit plugins to define their customs behaviours, like, for instance, switch color theme. This event could be binded with key like openbox do for ToggleMaximize. (not currently implemented)
> 
> i will look at the further patches when i can. :) thanks!
> 

0003-add-a-frame-trigger-function.patch doesn't apply cleanly, is there a patch missing in between?

(In reply to comment #26)
> 0003-add-a-frame-trigger-function.patch doesn't apply cleanly, is there a patch
> missing in between?
> 
The order is from origin/plugin branch : (in my repos)
0001-Implement-exemple-of-trigger-function-use.patch
0002-Fix-bug-that-openbox-don-t-draw-title-on-new-window.patch
0003-Improve-interface-by-removing-display-screen-and-ren.patch
0004-Remove-keep-border-option.patch
0005-keepborder-is-an-theme-option-now.patch

Number can be different.

Created attachment 1686 [details]
Fixed patch story

Last update can be downloaded on git :

git://repo.or.cz/openbox/manual.git

the branch used is plugin.

I had done folowing things:
   - creation of the frame top level window
   - map/unmap of top level and client windows (can have pre/post funcs for the
     engines) (engine should not deal with ignore_unmaps or grabserver)
   - reparenting of client into/out of the frame top level window
   - flashing end-time should come from openbox
   - iconify animation should move to compmgr (remove animation)
   - adjusting the focus_cycle_indicator
   - consistent use of "engine" vs "plugin".  prefer "engine" everywhere
   - combine set_client_area and update_layout
   - also moved boolean in update_layout should probably stay (or maybe
     moved/resized can both be implicit if we pass a rect and "fake"
     boolean only, by merging in set_client_area) Rect was added to
     update_layout
   - avoid using Rect as return type or fn parameter, use a Rect* in the 
     arguments instead, avoids excessive copying of rects consistant
     render_plugin->func() vs just func() : rect and strut are copied to the
     pointer argument.
   - make a create/destroy_event_window() function which everyone including
     engines can use that wrap XCreate/DestroyWindow and also calls
     add/remove_window for the event hashtable, pass fn pointer for this to
     engine (partiali done by code duplication)
   - if we need to. window_map should remain static

First draft of inclusion of popup is in plugin_experimental.