Bug 161122

Summary: Plasma::Svg::Private::findInCache creates invalid pixmap and X errors when size is invalid
Product: [Unmaintained] plasma4 Reporter: David Benjamin <davidben>
Component: generalAssignee: Plasma Bugs List <plasma-bugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description David Benjamin 2008-04-21 20:55:16 UTC 
Version:            (using Devel)
Installed from:    Compiled sources
OS:                Linux

(Sorry if this should have been reported to ksysguard instead; I wasn't sure.)

Steps to Reproduce:
1. Open ksysguard.
2. Switch to System Load tab

Results:
The following appears in the terminal.

QPixmap: Invalid pixmap parameters
QPainter::begin: Cannot paint on a null pixmap
QPainter::save: Painter not active
QPainter::setRenderHint: Painter must be active to set rendering hints
QPainter::setRenderHint: Painter must be active to set rendering hints
QPainter::restore: Unbalanced save/restore
QPainter::end: Painter not active, aborted
X Error: BadValue (integer parameter out of range for operation) 2
  Major opcode: 53 (X_CreatePixmap)
  Resource id:  0x0
X Error: BadDrawable (invalid Pixmap or Window parameter) 9
  Major opcode: 55 (X_CreateGC)
  Resource id:  0x420043c
X Error: BadGC (invalid GC parameter) 13
  Major opcode: 60 (X_FreeGC)
  Resource id:  0x420043d
X Error: BadDrawable (invalid Pixmap or Window parameter) 9
  Extension:    155 (RENDER)
  Minor opcode: 4 (RenderCreatePicture)
  Resource id:  0x420043c


I've traced the problem with gdb to Plasma::Svg. Specifically, the private findInCache() method supplies a default size of QSizeF(), which constructs an invalid size. In the case when elementId is empty, this size is eventually used (after some conversions to QSize, etc.) to create a QPixmap on line 196 of kdebase/workspace/libs/plasma/svg.cpp:

p = QPixmap(s);

This results in the QPainter and QPixmap errors, and presumably the X errors at perhaps a later point (but I couldn't tell for for sure with the X errors, since they seem to be asynchronous, and I don't know how to trace those with gdb).
Comment 1 Aaron J. Seigo 2008-04-21 21:32:41 UTC 
panel-devel@kde.org is often a more fluid forum for developer discussions =) fix to be committed shortly here.
Comment 2 Aaron J. Seigo 2008-04-21 21:32:54 UTC 
SVN commit 799536 by aseigo:

avoid painting empty pixmaps.

BUG:161122


 M  +11 -2     svg.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=799536
Comment 3 David Benjamin 2008-04-22 00:34:01 UTC 
Hrm, that seems strange. Because then the default argument for size in findInCache(), which is also the one used in Plasma::Svg::paint(), makes the function do nothing (after some complex trickery and cache lookups to verify it cannot do anything).

Is the intent rather to use Plasma::Svg::Private::size when no size is given? That would seem to make sense, and adding a small snippet to the effect
(e.g. something like...
if (!s.isValid()) {
    s = this->size.toSize();
}
...which is probably not a good fix since this messes up the cached id and probably belongs at the beginning of the function anyway)
does make ksysguard draw a background on the plot, which I can only assume was the intended purpose. It doesn't seem to be a mistake on ksysguard's part either (kdebase/workspace/ksysguard/gui/SensorDisplayLib/SignalPlotter.cc, line 577); at least, it looks a reasonable use. (Although I'm sure why they're using Plasma. Meh.)

Is this what Plasma::Svg is supposed to do? I'm not familiar at all with this API. (Otherwise I would have tried to attach a patch earlier.)

(I probably should have mentioned my confusion in the bug report in the first place... I thought I did, but I guess it got lost somewhere on the way from my head to my fingers. :-D)
Comment 4 Aaron J. Seigo 2008-04-22 00:56:20 UTC 
this is internal API, so it tends to favour utility over clarity a bit. the default argument is to be used primarily when painting just an element from the svg; the size is not known before hand, so we have to query the svg document first.


in any case, i'm moved the code around a bit more and removed the nasty #if's that were recently introduced (and introduced at least one bug =/)

> Although I'm sure why they're using Plasma.

to avoid having to reimplement Plasma::Svg and Plasma::Theme themselves.

anyways, if there's further discussion on this, please please take it to panel-devel@kde.org. b.k.o just sucks for these  kinds of conversations.