Version: (using Devel) Compiler: gcc (Gentoo 4.3.2-r3 p1.6, pie-10.1.5) 4.3.2 OS: Linux Installed from: Compiled sources This one is a bit tricky, so please bear with me. Try to run okular --unique file:///full/path/test.dvi#src:834test.tex followed by okular --unique file:///full/path/test.dvi#src:834test.tex on a multipage dvi document that contains source references. Pick a reference that appears not on the first page of the document (i.e. a high line number, e.g. 834 in my example). Note that due to bug #205076 you will need to use fully qualified urls to reproduce this bug. On the first call, the document gets correctly opened at the approximate location of the source reference. The second call reloads the document, but jumps to the first page of the document instead of to the location of the source reference. This also happens if you change the line number of the source reference in the second call (no matter what you change it to). As an aside, note how the correct page number is displayed at the bottom but the wrong page is shown after the second call. The vertical scrollbar position is also wrong. I have spent a lot of time trying to debug this and have narrowed down the source of the problem to a particular piece of code in PageView::slotRelayoutPages(), around the call to center(geometry.left[...]) in part 4) of this method (the call is in line 3022 of ui/pageview.cpp). On the first call of okular, this code gets eventually triggered and further down the track correctly sets the viewport corresponding to the given source reference. (There is always an earlier attempt at setting that viewport which fails since the viewport geometry has not been adapted yet and leads to an invalid (inf) normalizedY coordinate. I am not sure if that is a bug as such, though). On the second call of okular, however, no new PageView is constructed (obviously, because the document is opened via dbus), so some missing(?) resize code [see below] somehow leads to this center call not being made. This means that the viewport corresponding to the source reference has an invalid (inf) normalizedY coordinate when it is first set and different to the first call it is not set again later. The easiest way to see this happening is to put some debug code into Document::setViewport. By trial and error I have found a further indicator that the problem has to do with resize: Put the (completely irrational) call m_pageView->viewport()->resize(0,0); after the openUrl call in Part::openDocument( const QString &doc ) and now the document gets openend at least on the correct page when okular is called the second time. The viewport is still slightly wrong, though, so this is not even a workaround. This was just the only way I could find to get the center call mentioned above triggered. I couldn't find a way of telling okular to "dirty" the whole viewport although its geometry has not changed [this is my interpretation and may be way off the actual problem]. I am happy to help debug this further if needed. Incidentally, I found this bug and also bug #205076 while implementing forward search for pdf files using synctex. I will post my code to okular-devel later. Cheers, Jochen
Please verify whether it works in KDE 4.3.x or not. Current trunk potentially has another issue which could be misleading you in this.
Good call, Pino! I have tracked down the problem to the changes introduced in revision 989285 (i.e. the switch to QAbstractScrollArea). In revision 989284 forward search works just fine. This stuff is a bit beyond my knowledge of Qt, though. Any hints what I could be looking for? Cheers, Jochen
Okay, I might have found the problem. In PageView::slotRelayoutPages(), part 4) [line 3003ff in ui/pageview.cpp in trunk)] is only executed if ( fullWidth != contentAreaWidth() || fullHeight != contentAreaHeight() ) Commenting out this condition makes forward search work. It seems as though part 4) is also needed when the size has not changed at all, but the document has been reloaded. Not sure about the correct condition to use, though. Any suggestion, Pino? Now that this part works for me, I am encountering yet another problem. The pageview correctly changes to the requested viewport, but the coordinates that are being requested seem wrong (the normalizedY coordinate is typically very close to zero and negative!). I'll investigate this further and open another bug report once I know where this is coming from (should be something in the source anchor code). Cheers, Jochen
How did further investigation ended up?
I can't reproduce the problem with the wrong normalizedY coordinate anymore. Maybe it got fixed somehow in the meantime. So, to fix this bug for me, the if condition mentioned in comment #3 needs to be removed. The respective code needs to be executed also if the size hasn't changed but the document has been reloaded in the meantime. I don't know if there is an easy way to test for this rather than to remove the condition all together. Removing it doesn't seem to have any noticeable side effects on my system.
Could you please attach the necessary files to help me reproducing this?
Created attachment 39637 [details] dvi file showing the issue, see below for instructions
Created attachment 39638 [details] pdf file showing the issue, see beolw for instructions
Created attachment 39639 [details] synctex data for test case
Created attachment 39641 [details] synctex data for test case
Created attachment 39642 [details] latex source for the above
Above is a somewhat minimal test case for this bug. The dvi file can be tested stand-alone, the pdf file and the synctex.gz file need to be in the same folder to test. The .tex file is for your info and not strictly needed to test this. Run okular --unique test.{dvi|pdf}#src:33test.tex and the file should open roughly in the middle of page 3. Follow up with okular --unique test.{dvi|pdf}#src:57test.tex and you will see the view changing to page 1 with the page indicator flipping to page 5 (as it should). The problem is that the viewport does not get updated correctly (see comment #3 and comment #5). The file should be self-explanatory once you see it, so you will be able to create more sophisticated tests with it. Incidentally, it may be that bug #199184 is essentially the same bug since the workaround proposed there (press Ctrl+F triggering the search bar which also updates the viewport) works here as well. The reporter of that bug doesn't mention source references, though. Maybe the problem occurs as well if the dvi/pdf file itself gets changed while viewing it (I can not trigger that on my system, though)? Or the reporter of bug #199184 neglected to mention forward search?
Can you check if this fixes the problem for you? Index: ui/pageview.cpp =================================================================== --- ui/pageview.cpp (revision 1070532) +++ ui/pageview.cpp (working copy) @@ -752,12 +752,15 @@ // invalidate layout so relayout/repaint will happen on next viewport change if ( haspages ) + { // TODO for Enrico: Check if doing always the slotRelayoutPages() is not // suboptimal in some cases, i'd say it is not but a recheck will not hurt // Need slotRelayoutPages() here instead of d->dirtyLayout = true // because opening a document from another document will not trigger a viewportchange // so pages are never relayouted QMetaObject::invokeMethod(this, "slotRelayoutPages", Qt::QueuedConnection); + d->dirtyLayout = true; + } else { // update the mouse cursor when closing because we may have close through a link and
Yes, with this change the pdf case works perfectly. In the dvi case, the viewport now gets updated to the correct page, however, the vertical position is still wrong. It always shows an area near the top of the page. I guess this is the aformentioned problem with normalizedY in the dvi case (see end of comment #3). This appears to be a different bug, though, probably to do with how normalizedY gets calculated in the dvi case vs the pdf case. When I tried to track this down before, I had the impression that this may have to do with order of execution. By the time normalizedY gets computed in the dvi source anchor code, the viewport dimensions are not correctly set, yet. Or something like that. Do you want me to open a separate bug for this or do you want to handle it in this report since it is related to the same changes that triggered this bug (i.e. it used to work in rev. 989284)?
That sounds like a different bug whose fix would be Index: generator_dvi.cpp =================================================================== --- generator_dvi.cpp (revision 1071422) +++ generator_dvi.cpp (working copy) @@ -156,9 +156,9 @@ SimplePageSize ps = m_dviRenderer->sizeOfPage( vp.pageNumber ); double resolution = 0; + if (ps.isValid()) resolution = (double)(pW)/ps.width().getLength_in_inch(); + else resolution = m_resolution; - resolution = (double)(pW)/ps.width().getLength_in_inch(); - double py = (double)anch.distance_from_top.getLength_in_inch()*resolution + 0.5; vp.rePos.normalizedX = 0.5; Works for you?
Yes, indeed, that fixes the dvi case. And even better, it led me to improve the pdf case a bit as well: --- generator_pdf.cpp.orig 2010-01-08 12:29:07.000000000 +1100 +++ generator_pdf.cpp 2010-01-08 14:05:21.000000000 +1100 @@ -1541,11 +1541,12 @@ if ( !viewport.isValid() ) return; - // TeX small points ... - viewport.rePos.normalizedX = (synctex_node_h( node ) * dpiX) / (72.27 * 65536.0 * document()->page(viewport.pageNumber)->width()); - viewport.rePos.normalizedY = (synctex_node_v( node ) * dpiY) / (72.27 * 65536.0 * document()->page(viewport.pageNumber)->height()); - viewport.rePos.enabled = true; - viewport.rePos.pos = Okular::DocumentViewport::TopLeft; + double px = (synctex_node_visible_h( node ) * dpiX) / 72.27; + double py = (synctex_node_visible_v( node ) * dpiY) / 72.27; + viewport.rePos.normalizedX = px / document()->page(viewport.pageNumber)->width(); + viewport.rePos.normalizedY = ( py + 0.5 ) / document()->page(viewport.pageNumber)->height(); + viewport.rePos.enabled = true; + viewport.rePos.pos = Okular::DocumentViewport::Center; return; } With this change the target gets positioned more accurately within the center of the current view which makes finding it easier in cluttered pages. The trick is the "+ 0.5" in the correct units. Don't ask me why but the dvi case does this as well ... So, with the changes from comment #13, comment #15 and this comment committed the bug can be finally closed as fixed IMHO.
SVN commit 1072172 by aacid: Make sure resolution is not inf Fixes part of bug 205084 CCBUG: 205084 M +2 -2 generator_dvi.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072172
SVN commit 1072174 by aacid: backport r1072172 | aacid | 2010-01-09 15:08:53 +0000 (Sat, 09 Jan 2010) | 4 lines Make sure resolution is not inf Fixes part of bug 205084 CCBUG: 205084 M +2 -2 generator_dvi.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072174
SVN commit 1072187 by aacid: rename delayResizeTimer to delayRelayoutTimer and always create it, not on first resizeEvent Use the timer for the delayed slotRelayoutPages call in notifySetup instead of a QueuedConnection Also set the dirtyFlag in notifySetup, that way if we get a set viewport very soon we still end up in the correct place Stop the timer if we get to slotRelayoutPages so we never relayout twice without a need to This should fix bug 205084 BUGS: 205084 M +16 -15 pageview.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072187
SVN commit 1072188 by aacid: backport r1072187 | aacid | 2010-01-09 15:37:38 +0000 (Sat, 09 Jan 2010) | 7 lines rename delayResizeTimer to delayRelayoutTimer and always create it, not on first resizeEvent Use the timer for the delayed slotRelayoutPages call in notifySetup instead of a QueuedConnection Also set the dirtyFlag in notifySetup, that way if we get a set viewport very soon we still end up in the correct place Stop the timer if we get to slotRelayoutPages so we never relayout twice without a need to This should fix bug 205084 BUGS: 205084 M +16 -15 pageview.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072188
SVN commit 1072189 by aacid: commit improvement to the location of synctex thingies by Jochen Trumpf CCBUG: 205084 M +5 -3 generator_pdf.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072189
SVN commit 1072191 by aacid: backport r1072189 | aacid | 2010-01-09 15:42:45 +0000 (Sat, 09 Jan 2010) | 3 lines commit improvement to the location of synctex thingies by Jochen Trumpf CCBUG: 205084 M +5 -3 generator_pdf.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1072191