I should really set up this machine with a real email client. Sorry again to Jon for the double copy; duplicate for the list's benefit.
On 18 February 2011 00:35, Jon Nordby <[email protected]> wrote: > On 17 February 2011 23:20, Andrew Chadwick (list subscriptions) > <[email protected]> wrote: >> Could I canvass people's opinion and review on the dockable-panels >> branch in the main git repo? If it's not too broken or weird, I'd like >> to rebase it into master before the next major release (i.e. after >> 0.9.1). Probably needs some fixups first though... >> > As I said in the forum and on the mailing list I think this change is > really useful. But there are some issues. Okay, I've pushed http://gitorious.org/mypaint/mypaint/commits/dockable-panels-updated and deleted the old dockable-panels branch to avoid confusion relating to the funky interactive rebases performed in this new one's history. dockable-panels-updated can currently be fast-forwarded onto master. > First of all, the license of gui/layout.py is set to GPLv3. MyPaint, > is GPLv2+ (LGPLv2+ for brushlib). While not incompatible, this would > require us to distribute MyPaint under GPLv3, effectively a license > change for downstream recipients. I would really like to avoid that, > so please change the license to GPLv2 (and use the standard MyPaint > license header). Fixed. It's now GPLv2 with I hope a sensible-looking header. > Also, a big change like this is impossible to review properly when > delivered in essentially one huge commit. I'll therefore only comment > on superficial things regarding the code. Please split the commit into > several smaller pieces so we can review properly, and squash in "fixup > commits". Squashed the old fixup commits, but introduced a few more fix-type commits. Personally I can cope with that. I've split things up in a way that works all the way along the history, but this sort of refactoring means that code gets introduced before it gets used if you do that. Still, it's presented in a fairly logical order, and you should be able to squash changes that hit only layout.py quite nicely and introduce it where you want in the history. > Code > - Several hundred lines of code is duplicated in layerswindow.py. We > can't have that. Fixed. > - Moving MainWindow from windowing.py to layout.py looks to be unnecessary, There's code introduced in layout.MainWindow that integrates quite tightly with the main layout manager. However, I've rearranged this now to be a mixin to keep the previous hierarchy intact. > - I do not see the benefit of instantiating the menu-bar, et.c from > MainWindow, instead of just letting it happen within DrawingWindow > like before Menu bar instantiation takes place in drawwindow.MainWindow, just as before. There are some framework related things in there because the layout manager needs to know where to find the UI it assembles. Sure, it doesn't have to be done with a factory callable, but it might be nice since we *do* want the tool widgets and subwindows to be created by factory calls. > - There is no status-bar at the moment, why does the code have logic for it? Potential future expansion. It's a normal thing to have in a UI, and there are plenty of actions that could go in it to keep tablet PC owners happy. > - There is dead code (commented out) several places that needs to be removed. Done. Apologies for that. > - I'm fairly certain it was not necessary to instantiate the > backgroundWindow or brushSettingsWindow before, please check this Checked. It is necessary. The reason you never saw the regression before is that all windows were in fact being instantiated at startup so that their positions could be set at startup. > Regressions > - Main window does not obey window manager, is set to floating, > maximized by default It should be a floating window. It can't be docked into its own sidebar. Not sure what you mean here. The window is no longer maximized by default. > - Subwindows defaults to minimum size instead of something sane This can be fine-tuned in the default prefs. Since all the tools default to appearing in the sidebar, sometimes with explicit sizes, and adopt that size > - Subwindows no longer show up in front of main window Hmm, cannot replicate here with Metacity, Compiz, or wm2. Testing in twm or evilwm shows the behaviour you mention, but I don't expect those WMs to do the right thing anyway since they don't respect the utility window hint as far as I can tell. What WM are you using? > Strange behavior > - When scaling the sidebar, the canvas follows somewhat, but not > completely. Same when toggling the sidebar Yeah. Not sure what the best way of approaching that is. > - Gtk color selector resizing behaves strangely (as you noted). > Because of this the default color selector is colorsampler > Please fix this so that we can keep the Gtk one as default. Admittedly, I don't like the GTK colour selector and I don't think it should be the default, but I'm justified when I say there's not much I can do here without rewriting it. The fault does not lie in the new code. The way we construct the thing, by assembling a normal gtk.ColorSelector window then pulling out the embedded gtk.RGB and other nearby widgets is very broken, and results in something with a funky size_request somewhere in the resultant widget tree. We should use gtk.RGB directly if we decide to keep this old colour selector. -- Andrew Chadwick _______________________________________________ Mypaint-discuss mailing list [email protected] https://mail.gna.org/listinfo/mypaint-discuss
