Code review comment for lp:~openerp-commiter/openobject-client/client-image-widget

Revision history for this message
Nicolas DS (zyphos) wrote :

In GTK client all widget are separated by view. And the "Tree.image" does NOT exist for now in GTK client.

This branch is keeping widget="image" for image in Tree view, Form view and many2many in Form view (of course it is a tree view embedded in a screen view that acts as a form widget.).

In GTK client, "Tree view" widget are a lot different from "Form" one indeed:
- You need to define a new kind of cell renderer for special content. (It's not as easy as Form view)
- GTK treeview widget is asking a LOT of time to refresh row and column. (2 times to show the widget and then each time the mouse pointer get over, get out of any cell.)

In any refresh with the standard widget you need to: get the image data, compute the pixbuf (GTK image), then rescale the pixbuf to fit into screen. Those operations are very CPU/bandwidth intensive, that's why those widget need to be cached (for 120sec defined in bin/widget/view/common_gtk.py @line 137).

The problem with caching is the fact that if the image change in DB, the client will see the change maximum 120 sec further, and not in real time. Real time is still working for the basic "Form.image" widget.

At first I tried to use the widget="image" for many2one, but it's impossible in widget definition in openobject to know where type of field data come from.
Indeed widget python code is called from:
client/bin/widget/view/form_gtk/parser.py @line 416
widget_act = widgets_type[type][0](self.window, self.parent, model, fields[name])

and "type" is not alway the original field type, indeed:
@line 401
type = attrs.get('widget', fields[name]['type'])

and the many2one image widget needs to search for a binary field to get the data from.

I don't understand your one2many thing, I didn't made any one2many widget, if you look into source you will see that many2one_image is inheriting from the many2one class. So even if the many2one original class is a one2many I am lost.

The only fixing I see could be a different caching time or avoiding this caching time. But the best solution would be to create some kind of "push on field content change" from server to the client. But this feature needs a persistent connection to the server.
http://en.wikipedia.org/wiki/Push_technology

Or another one is to store hash of binary fields to quickly see if the image content has changed without eating lot of resource (CPU/bandwidth).

Don't forget to have a look at branch description for more information how this branch is working:
https://code.launchpad.net/~openerp-commiter/openobject-client/client-image-widget

Thank you for your review.

« Back to merge proposal