bug 74702 - Issue with bool OutputDevice::IsNativeControlSupported(ControlType, ControlPart)
glogow at fbihome.de
Sun Apr 21 05:02:07 UTC 2019
Chiming in a little late.
Am 15.04.19 um 09:50 schrieb Kaganski Mike:
> On 15.04.2019 10:44, Adrien Ollier wrote:
>> OutputDevice::AcquireGraphics() is a pure virtual function, so we cannot
>> know in the general case what it does.
>> And what is written is not really equivalent to what you wrote because
>> if AcquireGraphics() is false, there is not a second check for
>> mpGraphics. That's why it is an issue.
> AcquireGraphics() is not just "any function". Despite someone could of
> course implement it to play poker, its purpose is to acquire mpGraphics
> and return if it succeeded. Failing that is programmer's error breaking
> contract. There's no use to introduce that kind of checks here. Please
> see existing implementations, like VirtualDevice::AcquireGraphics() in
This is not the nicest API - probably the bool function should be renamed, or
also return the SalGraphics* for consistency; but then naming is hard...
$ git grep AcquireGraphics include/ vcl/inc/
include/vcl/outdev.hxx: virtual bool AcquireGraphics() const = 0;
vcl/inc/salframe.hxx: virtual SalGraphics* AcquireGraphics() = 0;
vcl/inc/salprn.hxx: virtual SalGraphics* AcquireGraphics() = 0;
vcl/inc/salvd.hxx: virtual SalGraphics* AcquireGraphics() = 0;
I was interested in the fact that the bool variant is const, which is a lie.
Looking at the code it's not true and in fact ~ half of OutputDevice's member
variables are mutable. That's probably done to do some lazy init for other const
functions, quite probably all the const font based ones. Don't know if it buys a
lot of time for some usage, or should be factored out somehow... Anyway, I did:
$ git log -G AcquireGraphics include/vcl/outdev.hxx
Author: Chris Sherlock <chris.sherlock79 at gmail.com>
Date: Wed Apr 23 00:57:50 2014 +1000
Rename VCL's ImplInitGraphics to AcquireGraphics
Turns out, we don't try to initialize a graphics context, much less
*acquire* one. e.g. Window instances can have many frames (subwindows),
of which one some are really being used at any time so we try to
"steal" one of the graphics contexts from the frame to use ourself,
later on that frame will steal it from someone else, etc.
This has nothing to do with the const, but explains the clash - kind of.
Normally the bool variants will - at some point - call
mpGraphics = ...->AcquireGraphics();
so much about the background stuff I was looking into.
Back to the original mail.
$ git grep "mpGraphics.*AcquireGraphics" vcl/
if ( mpGraphics || AcquireGraphics() )
if ( !mpGraphics && !AcquireGraphics() )
... 69 times
shows this idom is used some times in vcl. And IMHO it should be everywhere
if ( !AcquireGraphics() )
if ( AcquireGraphics() )
All the three bool variants start with:
$ git grep "bool.*::AcquireGraphics" vcl
vcl/source/gdi/print.cxx:bool Printer::AcquireGraphics() const
vcl/source/gdi/virdev.cxx:bool VirtualDevice::AcquireGraphics() const
vcl/source/window/window.cxx:bool Window::AcquireGraphics() const
bool *::AcquireGraphics() const
if ( mpGraphics )
The additional mutex check in debug builds is nice too.
For security I would rename the variants to Do / Impl and add an inline to the
OutputDevice header like so the "header" can't be stripped accidently:
inline bool OutputDevice::AcquireGraphics() const
if ( mpGraphics )
That also prevents the function call, if that really matters here.
Anyone want to make this an easy hack?
Revert the function name to ImplInitGraphics?
More information about the LibreOffice