Version: (using KDE KDE 3.5.7) Installed from: SuSE RPMs Compiler: gcc (GCC) 4.1.2 20061115 (prerelease) (SUSE Linux) Target: i586-suse-linux Configured with: ../configure --enable-threads=posix --prefix=/usr --with-local-prefix=/usr/local --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib --libexecdir=/usr/lib --enable-languages=c,c++,objc,fortran,obj-c++,java,ada --enable-checking=release --with-gxx-include-dir=/usr/include/c++/4.1.2 --enable-ssp --disable-libssp --disable-libgcj --with-slibdir=/lib --with-system-zlib --enable-shared --enable-__cxa_atexit --enable-libstdcxx-allocator=new --program-suffix=-4.1 --enable-version-specific-runtime-libs --without-system-libunwind --with-cpu=generic --host=i586-suse-linux Thread model: posix OS: Linux Hello! First of all, I'd like to thank you guys for your work on KPDF. This is a wish and a solution at the same time. I decided to use KPDF in a presentation and I also wanted to use my remote control to go back and forth and to switch between windowed and full screen modes on my laptop. The first two function were available via DCOP, the last one wasn't. So I added it (actually, there may be some redundancy since the code from slotShowPresentation and slotHidePresentation was used in slotShowHidePresentation but I didn't want to change too much in your code structure :-) ). In the attachment you'll find 3 patch.diff files and a profile for IRKick.
Created attachment 21187 [details] dcop.h.patch.diff
Created attachment 21188 [details] part.h.patch.diff
Created attachment 21189 [details] part.cpp.patch.diff
Created attachment 21190 [details] kpdf.profile.xml
solved by the reporter
Actually, some other functions are needed in full screen mode. Let's say there are some graphics and even in full screen mode all their details can't be viewed but if they are zoomed in it'll be just fine. Therefore "Zoom In", "Zoom Out" (maybe "Fit to Page Width", "Fit to Page", too) have to be accessible in full screen mode (acroread provides them all, evince some of them). But there is a problem: code redundancy. PageView and PresentationWidget don't share any zoom methods which means PresentationWidget has to reimplement the same zoom methods as PageView. Since they don't inherit anything from each other it is a major refactoring problem. There are several ways how to solve this. Only one view class is needed. Maybe another PageView has to be implemented where two modes are specified: normal and full screen. It needs to be discussed 'cause such a problem is a major flaw.
> Actually, some other functions are needed in full screen mode. Zoom support is bug 107452. And please, if the bug says about something, don't add comment about something totally different. Your original request was for adding a DCOP method to toggle the presentation mode. For some not really clear reason you closed it as "solved by reporter" (wtf?!, especially there were no commits about the patch that should be review, btw), then reopened it for something totally unrelated to the bug. Could you please use the bug tracking tool properly? Thanks.
Sorry, I wasn't clear about what I wrote about zoom methods: it is an addition to what is reported, read "I would like to use my remote control to go back and forth and to switch between windowed and full screen modes __and to zoom in and out in full screen mode__". Since it's not possible to edit the report I didn't want to open another bug to add just that so it is very much related to the reported bug ("KPDF + infrared remote control (for presentations)") and not totally different. I would like to add zoom methods myself and provide another patch but it's not that simple without major refactoring if it has to be done properly. As you can see the lack of zoom methods in full screen mode easily leads to the discovery of a problem in the application design. Therefore I wanted to discuss this before writing any patches. I hope now it's clear _what_ is said in the previous comment and _how_ it is related to the report. About closing and reopening: my bad (I thought with the provided patch there was no need to do anything further except for reviewing it which can be done within 30 seconds, really).
And I really hope you can understand that the zoom support for presentation mode is _already_ reported, no matter if you want remore control for it (how can you ask for "remove control for presentation mode zoom" if there's no zoom possibility for presentation mode?). So, comments on that go to bug 107452, otherwise it becomes hard for us track comments in different bugs. And about the patch reviewing: not all the people had 30 seconds free in the 2 minutes when you open and after that closed this report. If you want to help, we're here for accepting any kind of contribution, but please don't make our job hard.
Of course, I understand it. A bug is reported when something is missing. I couldn't switch between windowed and full screen mode so I added it. If there's no zoom possibility in presentation mode it can be added. If two bugs have something in common it's not a tragedy when a comment on one bug has also something to do with another bug. I don't say "let's throw them together to make your job harder", I say "in this report something is needed which can be also reported elsewhere". The set of missing features for the bug 107452 is a subset of missing features for this bug. That is all there is to it. But if we want to be really strict here it's more than just what's needed for the bug 107452. It's not only about zoom methods, it's more general and about software architecture of KPDF. Is there any official and extensive documentation about it? Maybe some UML diagrams? And where can this discussion be continued 'cause comments on the bug 107452 wouldn't be the best place either? Should we open another bug "Problems with software architecture of KPDF"? I thought it might be not possible to comment on a bug but it is still possible for a developer to review and apply a patch after a report has been closed. If comments are what you need the bug is reopened for that now :)
Hei Pino, don't scare my contributors :-P @patch: Sorry i've been without internet last week and still without a reliable connection, i'll look at your patches as soon as possible.
SVN commit 704875 by aacid: got the necessary 2 seconds to review the patch. Thanks and sorry for taking so long. Changed method's name to one i like better I'm not sure what are we supposed to do with the profile for IRKick, i guess just ignoring it :D BUGS: 148026 M +1 -0 dcop.h M +7 -0 part.cpp M +1 -0 part.h --- branches/KDE/3.5/kdegraphics/kpdf/dcop.h #704874:704875 @@ -29,6 +29,7 @@ virtual void slotNextPage() = 0; virtual void slotGotoFirst() = 0; virtual void slotGotoLast() = 0; + virtual void slotTogglePresentation() = 0; }; #endif --- branches/KDE/3.5/kdegraphics/kpdf/part.cpp #704874:704875 @@ -997,6 +997,13 @@ delete (PresentationWidget*) m_presentationWidget; } +void Part::slotTogglePresentation() +{ + if ( !m_presentationWidget ) + m_presentationWidget = new PresentationWidget( widget(), m_document ); + else delete (PresentationWidget*) m_presentationWidget; +} + void Part::slotPrint() { if (m_document->pages() == 0) return; --- branches/KDE/3.5/kdegraphics/kpdf/part.h #704874:704875 @@ -112,6 +112,7 @@ void slotShowLeftPanel(); void slotShowPresentation(); void slotHidePresentation(); + void slotTogglePresentation(); void close(); // can be connected to widget elements void updateViewActions();
> Changed method's name to one i like better Actually, that was the first name I used and I changed it to slotShowHidePresentation to follow the existing naming scheme ;-) > I'm not sure what are we supposed to do with the profile for IRKick, i guess just ignoring it :D ;-) that you can but it would be more useful to provide the profile when kpdf is installed (like those of amarok, kaffeine, kdetv, konqueror, kscd or noatun which are provided in respective packages and installed in $KDE_HOME/share/apps/profiles/). Then IRKick can provide the service for kpdf right after installation. In other words, the profile should be in the kpdf installation package.