Issue 107839 - Problem to select slides with linked pictures, also in DRAW
Summary: Problem to select slides with linked pictures, also in DRAW
Status: CLOSED FIXED
Alias: None
Product: Impress
Classification: Application
Component: ui (show other issues)
Version: OOo 3.2 RC1
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.2
Assignee: wolframgarten
QA Contact: issues@graphics
URL:
Keywords: oooqa, regression
Depends on:
Blocks: 99999
  Show dependency tree
 
Reported: 2009-12-22 17:35 UTC by asmodis
Modified: 2017-05-20 10:24 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
Test kit (134.51 KB, application/x-compressed)
2009-12-23 08:39 UTC, Rainer Bielefeld
no flags Details
Minimal patch to handle linked graphics like unlinked ones (689 bytes, text/plain)
2009-12-28 15:58 UTC, Armin Le Grand
no flags Details
2nd minimal patch (1.14 KB, text/plain)
2010-01-06 11:58 UTC, Armin Le Grand
no flags Details
3rd possible fix (1.65 KB, text/plain)
2010-01-06 13:17 UTC, Armin Le Grand
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description asmodis 2009-12-22 17:35:11 UTC
Do the following: 

- Create a presentation
- Add a linked picture
- Copy the created page 30x 

Result: 
In the slide sorter view it becomes impossible to 
select more than one slide. Selection i.e. four 
imidiately unselects them. Additionally the corner 
a around selection flackers. 

In the normal view it becomes impossible the scroll the 
slide preview on the left as it imidiately jumps back after 
scrolling. 

Reproducable on various platforms XP, OpenSuse, Kubuntu 
and on various PCs with different hardware, and also 
reproduced by different people. 

A real showstopper and 3.2 should not be released before 
this is fixed.
Comment 1 Rainer Bielefeld 2009-12-23 07:16:35 UTC
Not P1.
I believe I know that problem, but until now I only saw that with very big
presentations. I will test as reported.
Related to Issue 107605?

For reported effect "In the normal view it becomes impossible ..." we already
have Issue 96487 
Comment 2 Rainer Bielefeld 2009-12-23 08:03:37 UTC
Steps how do reproduce analogue to attached sample file.

1. Create new presentation "without frills"
2. In Drawing Toolbar click button "From File" to insert picture from file
3. Select .png from sample kit and insert it linked.
4. click into Slides pane outside any slide
5. <cntrl>+<a> to slect all
   Slide 1 will gett border
6. <cntrl>+<c>, then <cntrl>+<v> to copy paste slide
   dialog appears
7. Select 'Insert after', <ok>
   New slied(s) will be inserted
8. redo from step 4 until you have 16 slides
9. redo from step 4 
   expected: everything works normal
   actual: after <cntrl>+<a> all slides get border as expected, but then border 
           disappears immediately, slides become unselected

In Slide Sorter Pane I see that impossibility to select all slides beginning
with 8 slides in it.

Indeed, it seems that the linked pictures cause the problem. If I start a new
test from step 1, but with embedded pictures, I did not see the problem even
with 256 slides or more.

Now I doubt that this problem is the same as in Issue 107605, because I see THAT
problem also for slides with embedded pictures.

I see the completely identical problem in the "Pages" pane in DRAW.

Worked fine with "2.4.1  Multilingual version German UI WIN XP:
[680m17(Build9310)]". 

Compared to 2.4, performance for copy paste slides is very very poor on my PC.

It's a serious problem, I see it as a 3.2 blocker.
Comment 3 Rainer Bielefeld 2009-12-23 08:39:47 UTC
Created attachment 66774 [details]
Test kit
Comment 4 uwe.luebbers 2009-12-28 12:47:56 UTC
Assigned owner and adjusted target
Comment 5 clippka 2009-12-28 14:47:22 UTC
I'm not able to reproduce this issue on my xp workstation (amd dual core 2ghz,
3gb ram, office (non gamer) graphics card), even with 32 slides I have no
selection problem.

I'm currently debuging to see if there is something wrong with the graphic
caching...
Comment 6 Armin Le Grand 2009-12-28 14:53:57 UTC
AW->CL: Checked. Problem is that there is not really an asychronious loading for
linked graphics (and never was, even in old versions). When a linked graphic is
to be shown, ViewObjectContactOfGraphic::createPrimitive2DSequence uses the
helper impPrepareGraphicWithAsynchroniousLoading (instead of the synchronius
version). Independent from this, the async and the sync version uses
SdrGrafObj::ImpUpdateGraphicLink() to load the linked graphic. This works, but
uses internal stuff which calls SdrGraphicLink::DataChanged(..) which triggers a
SetChanged (correct for model data) which itself triggers a ActionChanged().

That ActionChanged() is triggered during repaint which leads to repaint races.
Besides that the PagePane View has IsSwapAsynchron() set to false, thus forcing
to load all the time anyways. Somehow for some count the loaded swapped linked
graphics get forced to be unloaded again. From that amount (ca. 6, settable as
graphics cache in the tools/settings somewhere) the whole system seems to need
longer and longer to get to an ending state.

The error is that indirectly an invalidate gets triggered during paint. I tried
to move

			if(rGrafObj.IsLinkedGraphic())
			{
				// update graphic link
				rGrafObj.ImpUpdateGraphicLink();
			}

from the preparators (impPrepareGraphicWithAsynchroniousLoading and
impPrepareGraphicWithSynchroniousLoading) to
ViewObjectContactOfGraphic::doAsynchGraphicLoading(), thus using
SdrGrafObj::ForceSwapIn() for synchronious load and
SdrGrafObj::ImpUpdateGraphicLink() for asynchronious, but outside paint. This
works better, but such a change is hard to predict.
Comment 7 clippka 2009-12-28 15:03:33 UTC
my only idea for a fix would be to introduce a kind of resource manager (which I
already have in the queue for embedded sounds) which would detect that the same
graphic is used on every slide so it would be loaded once and only once.

But this would only fix a theoretical issue as usualy the user would have
multitple slides with different images (or put one image as a background image
on the master page which would get reused already).

I assign this issue to af for know as he is the maintainer of the slidesort

cl->af: please consult with aw if there is a possibility to have a fix for this
for OOo 3.2. If I recall correctly, this will be fixed anyway with your
slidesorter rework targeting at OOo 3.3++ ?
Comment 8 Armin Le Grand 2009-12-28 15:58:52 UTC
Created attachment 66868 [details]
Minimal patch to handle linked graphics like unlinked ones
Comment 9 Armin Le Grand 2010-01-06 11:23:54 UTC
AW: More infos:
- The problem does happen in slightly changed form in a DEV300 m66. The update
of the SlideSorter also gets slower, but gets to an end faster in comparison.
- The problem can not be avoided by increasing the tools/options/memory options
(or i could not find out, how to do that)
- With the proposed fix, the main view handling gets better. The SlidePane still
flickers and needs long time for update.

Guess: The main view gets better since the linked graphics can be loaded
asynchron with the proposed fix. The SlidePane still loads synchron since
IsAsynchronGraphicsLoadingAllowed() is set to false, thus still all graphics get
loaded all the time since the SlidePane repaint seems to repaint all visible
previews (may have to do with invalidates due to the loading again).

When loading synchronous for SlidePane this can be detected since the
ObectContact is a preview (true == GetObjectContact().IsPreviewRenderer()). In
that case, the graphic may be loaded only temporarily (as already done for
printing) in that case. Testing...
Comment 10 Armin Le Grand 2010-01-06 11:56:35 UTC
AW: Works much better with the 2nd change.
Tested with changes and many pages linked and non-linked graphics. Non-linked
are much faster, so i got the idea to use IsPreviewRenderer also in
impPrepareGraphicWithAsynchroniousLoading and
impPrepareGraphicWithSynchroniousLoading for rGrafObj.ForceSwapIn() calling
where rObjectContact.isOutputToPrinter() is checked. This does not change
anything, so removed again.
Adding patch so far.
Also will test another idea: Remove all Changed() and ActinChanged() calls from
ImpUpdateGraphicLink. Should not be needed nowadays...
Comment 11 Armin Le Grand 2010-01-06 11:58:35 UTC
Created attachment 67027 [details]
2nd minimal patch
Comment 12 Armin Le Grand 2010-01-06 13:01:29 UTC
AW: Indeed, when commenting in SdrGrafObj::SetGraphic(..) the lines
//	SetChanged();
//	BroadcastObjectChange();
all works well. The problem is indeed that SdrGrafObj::SetGraphic(..) gets
called from SdrGraphicLink::DataChanged(..).
Test shows that it's even enough to comment the 2nd line, thus the problem
really is the model change broadcast. The 1st line just leads to more repaints.
All this is triggered by SdrGrafObj::ImpUpdateGraphicLink() calls. AFAIK the
SlidePane refreshes on ModelChange the page preview for the page mentioned in
the hint. This re-creates the preview graphic and this needs to reload the graphic.
The tragedy here is that SdrGraphicLink::DataChanged(..) does not even INTEND to
change the model; it saves it's state and restores it in the most cases. This is
an UNWANTED model change (!)
What is (and was always) missing here is a SdrGrafObj::NbcSetGraphic(..). I
guess the one who added to rescue the mode state just did'nt want to add it.
Testing with adding it. Both commented lines will me moved to be not in the Nbc
method...
Comment 13 Armin Le Grand 2010-01-06 13:17:18 UTC
Created attachment 67032 [details]
3rd possible fix
Comment 14 Armin Le Grand 2010-01-06 13:24:02 UTC
AW: Added 3rd solution as patch.

AW: I think 3rd solution is the safest way and i propose it.
It only changes SdrGrafObj::SetGraphic to SdrGrafObj::NbcSetGraphic at
SdrGraphicLink::DataChanged where it is needed. There were model changes in
SdrGraphicLink::DataChanged before (when no graphic was set yet) but i think it
was wrong. Thus, this fix only changes the refreshes after swapin of a linked or
embedded graphic. None of the two situations justifies a model change. SlidePane
does not need a model-change based refresh, since it's preview was already
forced with correct swapped-in data and the preview stays valid.

AW: Still happening: the jumping around in the SlidePane, independent from
linked or non-linked graphics.
Comment 15 Armin Le Grand 2010-01-07 11:40:57 UTC
AW: This task handles two problems:
(a) The timing problem with linked graphics
(b) the 'jumping' when selecting in the SlidePane.

Since (a) is the critical part and i have a proposed fix and RC2 (which we
should use as a test) is in front of the door, I'll split this task:

- Takeover this task for (a) and add fix and task to CWS impress185 for RC2
- Wrtne another task for AF for (b)
Comment 16 Armin Le Grand 2010-01-07 11:46:28 UTC
AW: Submitted #i108126# as FollowUp.
AW: Adding patch tpo CWS impress185...
Comment 17 Armin Le Grand 2010-01-07 12:39:00 UTC
AW: Added and tested. Created files with linked, unlinked and mixed graphics,
all at least with 15 pages. Reloaded and exported to PDF. Works as expected.
Comment 18 Armin Le Grand 2010-01-07 13:50:45 UTC
AW->WG: Please verify.
Comment 19 wolframgarten 2010-01-08 08:18:31 UTC
Verified in CWS.
Comment 20 Rainer Bielefeld 2010-02-05 05:28:57 UTC
*** Issue 105273 has been marked as a duplicate of this issue. ***