Created attachment 10251 [details]
vcl speedup

accepted

just for curiosity, have you measured any speedup ? about four years
ago we had a "sophisticated" cache for GCs. I removed it completely. I
measured that there's no impact on performance, even on remote displays.

why do you consider this P2 ?

Sure; the problem is that doing a separate AlphaMask render for each
icon shows that this path is taking siginificant time; doing an strace
shows there is a large amount of redundant X server round-trip
traffic; taking a handful of milliseconds per icon: for the ~50 or so
visible icons that's quite a hit.

Of course - really, we need X-render support, but getting this right
first would be nice I guess.

As for P2 - I've no idea how priorities work, but my experience is of
bugs festering unassigned for a long time ;-)

> Sure; the problem is that doing a separate AlphaMask render for 
> each icon shows that this path is taking siginificant time; 
> doing an strace shows there is a large amount of redundant X 
> server round-trip traffic; taking a handful of milliseconds 
> per icon: for the ~50 or so visible icons that's quite a hit.

ok

> Of course - really, we need X-render support, but getting 
> this right first would be nice I guess.

sigh

> As for P2 - I've no idea how priorities work, but my 
> experience is of bugs festering unassigned for a long 
> time ;-)

unfortunately we have a severe scalability issue with this method :-)

Urk - as I suspected, I know nothing about GCs - and this patch in
fact quite badly screws up some image rendering.

eg. the attached image inserted into a writer document renders
awefully; I suspect the problem is that we want .foreground=1,
.background=0 and not the strange set of other options that the CopyGC
/ MonoGC give us by default;

Bother.

Created attachment 10268 [details]
sample image

Here is my strace of the DrawAlphaMask method in action:

[pid 20025] 1066062908.345510 access("get bitmap", F_OK) = -1 ENOENT
(No such file or directory)
[pid 20025] 1066062908.345697 write(6,
"6\20\2\0\336\1\240\2;\3\5\0009\0\240\2\0\0\0\0\r\2\37\0"..., 1104) = 1104
[pid 20025] 1066062908.346471 read(6,
"\16\0\214-!\0\240\2\0\0>\0\351\1\0\0 \0\0\0\336\1\240\2"..., 32) = 32
[pid 20025] 1066062908.346668 read(6,
"\1\0\300-\3\0\0\0$\0\0\0\1\0\0\1\377\377\377\377\0\0\0"..., 32) = 32
[pid 20025] 1066062908.346764 read(6,
"\177\200\353\0\177\200\353\0\0\0\267\10", 12) = 12
[pid 20025] 1066062908.346844 read(6,
"\1\20\301-\0\0\0\0H\0\0\0\5\0\27\0S\4\233\3\0\0\0\0\310"..., 32) = 32
[pid 20025] 1066062908.347021 access("done get bitmap", F_OK) = -1
ENOENT (No such file or directory)
[pid 20025] 1066062908.347176 write(6,
"5\20\4\0\342\1\240\2!\0\240\2\30\0\30\0007\0\4\0\343\1"..., 88) = 88
[pid 20025] 1066062908.347704 read(6,
"\16r\304-\342\1\240\2\0\0>\10\320\1\0\0E\0\0\0\30\0\0\0"..., 32) = 32
[pid 20025] 1066062908.347852 read(6, "\1\20\306-
\1\0\0\0\0\0\0\320\214\203\10\25\0\0\0\270["..., 32) = 32
[pid 20025] 1066062908.347953 readv(6,
[{"\276\367\276\367\276\367\276\367\276\367\276\367\276\367"...,
1152}, {"", 0}], 2) = 1152
[pid 20025] 1066062908.348369 access("done composite", F_OK) = -1
ENOENT (No such file or directory)
[pid 20025] 1066062908.348633 write(6,
"6\20\2\0\342\1\240\2;\3\5\0P\0\240\2\0\0\0\0\0\0>\0S\4"..., 1272) = 1272
[pid 20025] 1066062908.349125 access("done draw", F_OK) = -1 ENOENT
(No such file or directory)

Looks like there's something very odd happening; it _looks_ like we
push all the image data to the server, then get it back again, then do
the composite, then push it back to the server;

The 'access' syscalls are added thus:

@@ -1498,7 +1501,9 @@ void OutputDevice::ImplDrawAlpha( const 
                {
                        GDIMetaFile*    pOldMetaFile = mpMetaFile;
mpMetaFile = NULL;
                        const BOOL              bOldMap = mbMap; mbMap
= FALSE;
+                       access( "get bitmap", 0 );
                        Bitmap                  aBmp( GetBitmap(
aDstRect.TopLeft(), aDstRect.GetSize() ) );
+                       access( "done get bitmap", 0 );
 
             // #109044# The generated bitmap need not necessarily be
             // of aDstRect dimensions, it's internally clipped to
@@ -1685,11 +1690,13 @@ void OutputDevice::ImplDrawAlpha( const 
                                                }
                                        }
                                }
-
                                ( (Bitmap&) rBmp ).ReleaseAccess( pP );
                                ( (AlphaMask&) rAlpha ).ReleaseAccess(
pA );
                                aBmp.ReleaseAccess( pB );
+                               access( "done composite", 0 );
+
                                DrawBitmap( aDstRect.TopLeft(), aBmp );
+                               access( "done draw", 0 );
                        }
 
                        delete[] pMapX;

Any ideas ? [ it seems we have to do the CreateGC's for the new X
resources we create unfortunately ]

I think we do not have to create new GCs every time. If the CopyGC and
MonoGC from SalDisplay do not suffice, another buffered GC on the
SalDisplay should do.

As for the getting/putting images, i'll have to have a look.

Sigh; so - I'm not convinced this is really a problem whatsoever after
more careful inspection - at least, for my high-color display.

I've re-hashed the patch, this time to avoid the sequence: GetBitmap,
AcquireBuffer ... do composite ... -> DrawBitmap which was doing:

CreatePixmap, CopyPixmap [GetBitmap]
CreateImage, GetImage from Pixmap -> DIB [AcquireBuffer]
DIB -> drawable [DrawBitmap]

But with this patch - I believe does:
Create Image, GetImage from Drawable [GetBitmap]
<nothing> [ AcquireBuffer ]
DIB -> drawable [ DrawBitmap ]

Which sounds more efficent - but while (of course) perceptibly one
hopes it's improved, emperically the strace doesn't show anything too
amazing;

Anyhow; sorry for all this spam for such an ill-defined advantage.

Created attachment 10306 [details]
a different approach

commited in CWS vcl7pp1r3

verified changes are in CWS vcl7pp1r3

merged in 645m25s1