View Issue Details

IDProjectCategoryView StatusLast Update
0002761ScribusInternalpublic2005-11-05 12:40
ReporterringercAssigned Tofschmid  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformx86 LinuxOSFedora CoreOS Version3
Product Version1.3.2cvs 
Fixed in Version1.3.2cvs 
Summary0002761: Slow moving large vector groups - profiling data
DescriptionI've finally got around to doing some profiling to back up my impression that moving around groups of vector objects - like text that's been converted to outlines - is unreasonably slow.

I'm putting this up on bugs because I'm unlikely to get to look into speeding it up in the near future, and don't want to forget this.

Here's the top of the profiling summary from creating an A4 page of 12pt text, converting to outlines, dragging it around twice, then quitting:

Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total           
 time   seconds   seconds    calls   s/call   s/call  name    
 58.36    110.76   110.76    33074     0.00     0.01  ScribusView::setGroupRect()
 35.04    177.26    66.50 156940153     0.00     0.00  QGList::at(unsigned int)
  1.23    179.60     2.34 156586836     0.00     0.00  QPtrList<PageItem>::at(unsigned int)
Steps To Reproduce- Rebuild Scribus with the --enable-profile and --enable-debug args to configure (remember to "make clean").
- Launch Scribus
- Create an A4 doc
- Make a large text frame
- Fill with sample text
- Right click, "convert to outlines"
- Drag the sample text around a bit, dropping it a few times.
- Quit without saving.
- copy ~/gmon.out to the dir containing the Scribus binary
- run: gprof ./scribus | tee profile.out

and check out profile.out
TagsNo tags attached.
Patch

Relationships

related to 0002764 closedringerc Opening a gzipped doc with new encoding-correct readSla(...) takes FOREVER 
related to 0003821 acknowledged Metabug: Usability 

Activities

2005-10-24 19:02

 

prof_outlinetext.txt.gz (352,770 bytes)

ringerc

2005-10-24 19:42

reporter   ~0007200

At a guess, we're doing something like calculating the rect around the whole group once for each /element/ of the group rather than for the group as a whole. Something like that.

avox

2005-10-25 19:24

updater   ~0007202

Would help if we had a stack trace for the hotspots.
Can you provide a sample file so I can test with Shark on OSX?

2005-10-26 02:12

 

outlines.sla.gz (648,244 bytes)

ringerc

2005-10-26 02:24

reporter   ~0007212

Document attached; created as per "steps to reproduce" above.

I had a look at ScribusView::setGroupRect(...) and there's nothing obviously wrong with it in speed terms, especially for objects of zero-degree rotation. It's being called WAY more than I'd expect for the number of objects in the selection

With rotated objects I'm worried that the fpointarray is going to do a bunch of allocations, so it might be better done as:

FPointArray pb(4);
pb.setPoint(0, FPoint(...));
pb.setPoint(1, FPoint(...));
pb.setPoint(2, FPoint(...));
pb.setPoint(3, FPoint(...));

... but that shouldn't be the cause for this issue anyway, given that it only affects rotated frames.

Neither path seems to do any heap allocations or anything similarly horrifying.

ringerc

2005-10-26 02:26

reporter   ~0007213

That doc turns up a problem with the encoding-correct readSLA(..) I put in recently. It looks like QCString::operator+= is *REALLY* slow, much slower than QString::operator+= that was being used in the bad-encoding version.

I'll have to look at this, as opening the gzipped file takes FOREVER.

ringerc

2005-10-26 02:32

reporter   ~0007214

There are some 4400 objects in that test page; I think there would've been somewhat fewer in the one I used to generate the profiling data. Given that, it's it seems highly likely that setGroupRect(...) is being called for each item in the group, possibly every cursor movement (given the multiple of the number of objects), rather than just once for the group as a whole.

cbradney

2005-10-26 09:45

administrator   ~0007215

I guess part of the issue is its reliancy of its X,Y pos as the initial point. It'd be nice if its rectangle could be stored and then simply translated when used. In this way, when a new item is added to the group, or at the end of a "group add transaction", the site/shape of the group rectangle is calculated then, rather than for every move.

ringerc

2005-10-26 09:54

reporter   ~0007216

Yep. Similarly, if we get a selection class at any point, the selection should probably track its bounds the same way. Adding would be easy, but removing items would still require a re-scan. It'd be very quick and cheap anyway so long as it's not done for every single element.

cbradney

2005-10-26 12:29

administrator   ~0007219

Well, the selection class is one of the things I want to start, need some ideas how how we would want it to work though.

fschmid

2005-10-29 09:31

developer   ~0007250

Some improvements:
- setGroupRect is now only called once when moving a Group instead of calling it for every single Item.
- when moving a Group only the bounding box is now drawn.

ringerc

2005-10-30 08:14

reporter   ~0007254

I've just pulled the latest CVS; the difference is absolutely unbelievable. Thanks Franz!

My only suggestion now would be that from a usability perspective, it'd be ideal to draw the oulines of objects in a group / selection if there are only a relatively small number of them being dragged. This would make life easier for people doing things like positioning a small group of vector shapes.

It's hard to know when is the right time to stop drawing the oulines, since sometimes one complex vector shape is enough to slow things down really badly, and other times you can move 100 of them with no detectable slowdown.

Ideas?

fschmid

2005-10-30 08:57

developer   ~0007255

Maybe drawing the outlines only once into a QPixmap and then only moving that QPixmap would be a solution.

fschmid

2005-10-31 07:51

developer   ~0007276

Would this solution be acceptable?
- if there are < 20 Objects selected full Outlines are drawn
- if there are > 20 and < 40 Objects selected only their bounding boxes are drawn
- for any number of Objects greater than 40 only the Group bounding box is drawn

ringerc

2005-10-31 08:29

reporter   ~0007278

That seems sensible enough. If those numbers are constants stored somewhere easy to find (perhaps `static const unsigned int' members of the view?) then it'll be easy to tune later too.

fschmid

2005-10-31 10:41

developer   ~0007289

Done now, Constants are named:
moveWithFullOutlinesThreshold = 21
moveWithBoxesOnlyThreshold = 41

Can we resolve this bug now?

ringerc

2005-10-31 11:03

reporter   ~0007290

Sure. Thanks very much for that, it's been a funny performance corner case for a long time.

Issue History

Date Modified Username Field Change
2005-10-24 19:01 ringerc New Issue
2005-10-24 19:02 ringerc File Added: prof_outlinetext.txt.gz
2005-10-24 19:05 ringerc Description Updated
2005-10-24 19:42 ringerc Note Added: 0007200
2005-10-25 19:24 avox Note Added: 0007202
2005-10-26 02:12 ringerc File Added: outlines.sla.gz
2005-10-26 02:24 ringerc Note Added: 0007212
2005-10-26 02:26 ringerc Note Added: 0007213
2005-10-26 02:32 ringerc Note Added: 0007214
2005-10-26 02:32 ringerc Status new => acknowledged
2005-10-26 02:32 ringerc Relationship added related to 0002764
2005-10-26 09:45 cbradney Note Added: 0007215
2005-10-26 09:54 ringerc Note Added: 0007216
2005-10-26 12:29 cbradney Note Added: 0007219
2005-10-29 09:31 fschmid Note Added: 0007250
2005-10-30 08:14 ringerc Note Added: 0007254
2005-10-30 08:57 fschmid Note Added: 0007255
2005-10-31 07:51 fschmid Note Added: 0007276
2005-10-31 08:29 ringerc Note Added: 0007278
2005-10-31 10:41 fschmid Note Added: 0007289
2005-10-31 11:03 ringerc Status acknowledged => assigned
2005-10-31 11:03 ringerc Assigned To => fschmid
2005-10-31 11:03 ringerc Status assigned => resolved
2005-10-31 11:03 ringerc Resolution open => fixed
2005-10-31 11:03 ringerc Note Added: 0007290
2005-10-31 11:03 ringerc Fixed in Version => 1.3.2cvs
2005-11-05 12:40 cbradney Status resolved => closed
2006-05-15 18:13 christoph_s Relationship added related to 0003821