blend function urgently needed in kdelibs
thiago at kde.org
Tue May 22 18:52:01 BST 2007
Matthew Woehlke wrote:
>> 1) You're mixing the enums. ColorSpec is declared in a flag-like
>> manner, but it has no Q_DECLARE_FLAGS; BlendMode or BlendFlags (choose
>> one) is declared in a non-flag mode but has Q_DECLARE_FLAGS. You want
>> to review that.
>Thanks, but that's actually intentional, it should not be used as flags
>except by KColorPrivate (although Matt Nowell is doing his darndest to
>talk me out of that :-)). I guess making it 'not flags-like' would mean
>'making the values 1, 2, 3, ...'? (I want to confirm because I may end
>up going that way...)
That's what I meant. If you need to use the type in a flags-like manner
internally, you can use (1<<type) wherever needed. But this is minor.
As for BlendMode, you used Q_DECLARE_FLAGS, but the enum values do not
support operation in flags mode. Besides, it doesn't make semantic sense:
what would be a Replace, Multiply *and* Add blend?
>> 2) Please remove the protected function and the static inlines. They
>> have no business being in a public header file.
>Then where /do/ protected functions belong? (What if someone wanted to
Why would anyone want to subclass KColor? What information/manipulators
would you want to provide sub-classers that shouldn't be available to
If you have a valid case for that above, then it makes sense to have a
protected function. Otherwise, simply remove them.
In this case, I see no need for a protected "blend" function that doesn't
even seem to touch the private members.
Rule of thumb: if you have no immediate need or use-case for a function,
don't publish it. If the need you thought of actually shows up later, you
can add it at that time. Exception: virtual functions.
>I suspected someone would complain about the static inlines, the thing
>is I would like them to be available... are they OK non-inline? (It's
>not like they're going to change, though...)
No, they shouldn't be there at all. Not these, at least. Why do you want
to provide them? Who would use them?
If you want to make those functions present for your own code to be
readable, make them static non-members in your .cpp file.
>> 3) Constructors must be explicit or explicitly implicit. (i.e., add
>> the krazy mark if it's to be implicit)
>I must admit I'm not entirely familiar with what the difference is, how
>do I know which should be which? Is there a doc somewhere?
The two constructors:
KColor( const QColor& );
KColor( const KColor& );
are different in meaning. The latter one is a copy-constructor and it
generally makes no sense to make it explicit. So you can leave it as it
The QColor overload, however, is the question. The Library Code Policy
"For each constructor (other than the copy constructor), check if you
should make the constructor explicit in order to minimize wrong use of
Basically, each constructor that may take only one argument should be
marked explicit unless the whole point of the constructor is to allow
In this case, I would say that the whole point is to allow implicit
casting, so you want to tell krazy that you know it is implicit. Just add
an " // implicit" after it to tell krazy to go away.
>> 4) If you want implicit sharing, use QSharedDataPointer.
>See conversation with Matt Nowell, this is still in the air.
I have received no emails from Matt Nowell.
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: not available
More information about the kde-core-devel