[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: flake/canvasController
From:       Boudewijn Rempt <boud () valdyas ! org>
Date:       2010-09-14 8:37:47
Message-ID: 201009141037.47792.boud () valdyas ! org
[Download RAW message or body]

On Monday 13 September 2010, zander@kde.org wrote:
> On Monday 13. September 2010 09.51.57 Boudewijn Rempt wrote:
> > SVN commit 1174707 by rempt:
> > 
> > Explain the design of KoCanvasController a bit more
> > 
> > (And yes, there is more than one implementation of KoCanvasController,
> > it's just that not all of them are in the KOffice tree).
> 
> As flake is not yet frozen that is a dangerous proposition; I was already 
> prepared to refactor the code to avoid the unneeded spitting of classes.

There is no unneeded splitting of classes, so I'm glad you haven't spent time on that \
already.

> The docs give no indication why there is any need for a canvasController that 
> is not based on a QWidget. And I can't come up with any.

Because there are Qt-based widget sets that do not use QWidget, but QGraphicsItem. \
And we want KOffice to go there as well, without using QGraphicsProxyWidget (which is \
notorious for bad performance). So we cannot base all our canvas handling on \
QWidgets. 

This way, there's the flexibility to use QWidget-based canvases, QGraphicsItem-based \
canvases -- or something else, even. We discussed this at the last sprint -- moving \
away from a hard dependency on QWidget everywhere -- and this is the just the first \
step.

The next step is, of course, to make sure that applications can render and edit \
documents without the necessity of a KoView-based class (rendering works, editing \
probably not yet), and after that making sure that tools don't carry hard-coded \
QWidget-based option panes.

> As one of the maintainers of flake I would suggest that if some external 
> project has this need there should not be a solution inside of KOffice like we 
> have now; 3 classes that are split with no visible reason in any app I can 
> find.

Suggestion noted, but, well, we have more code in KOffice that only exists because it \
opens up some possiblity, and sometimes it isn't used at all, like the read-only tool \
feature. So that's not an argument against providing support for this kind of \
flexibility inside KOffice even if only external applications need it.

As I said above, I think it's a good thing for KOffice in general and something that \
should be pushed further along.

> Please provide a link to the sources of this app so at minimum the flake 
> maintainers can at minimum see what is going on and maybe write some better 
> documentation on the logic and justification of this splitting.

Sorry, I cannot do that. The application will likely be opened up when it is \
finished. It already contains LGPL header notices, but it has not been released yet \
by Nokia. But I don't have any say in this.

By the way, the KOffice community has decided that libraries do not have explicit \
maintainers,right? So talking about "The flake maintainers" is a bit weird -- and I \
am unsure here, but did you mean to imply that if such a group would exist, I \
wouldn't be part of it?

Anyway, I will try to improve the documentation even more, but I have a feeling that \
it will never be enough.

> I'm quite confused how this refactor managed to get through an API review, it 
> goes against all the work we did in the last year in cleaning up this library.

I think this patch is ok, and so thought the reviewers. The patch was posted, \
reviewed by Marijn and Thorsten and okayed. And that's the end of it, sorry. I don't \
think it goes against any of the cleaning-up work in flake either, but rather \
improves on it. 

-- 
Boudewijn Rempt | http://www.valdyas.org
Ceterum censeo lapsum particulorum probae delendum esse
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]