gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnustep-cvs] r31195 - in /libs/gui/trunk: ChangeLog ColorPickers/GS


From: Eric Wasylishen
Subject: Re: [Gnustep-cvs] r31195 - in /libs/gui/trunk: ChangeLog ColorPickers/GSWheelColorPicker.m Source/NSColorPanel.m
Date: Tue, 24 Aug 2010 14:49:32 -0600

Hi Fred,
The problems I was trying to fix here is that changing the color wheel
brightness and resize the color wheel window was quite slow with the
default color panel size, and got unbearably slow if you made the
window bigger.

On my computer (2.3 ghz core 2 duo, Windows) with my newly committed
code it takes 100ms to render the color wheel at its maximum size (I
added a max size for the color panel in this patch, previously it was
unlimited). Changing the rendering to use this line:

[bmp setColor: [NSColor colorWithCalibratedHue:h saturation:s
brightness:v alpha:A] atX: x y: y];

instead of current inline color space conversion and pixel
manipulation increases the time to 4.4 seconds! For reference, drawing
the same size wheel the old PS code took 5.3 seconds. So the message
send overhead is huge in this case. At the max size, the wheel is
474x476, or about 250k pixels, and given that the setColor: and
colorWithCalibratedHue: calls probably involve at least 5-10 sends,
we're looking at millions of message sends.

I agree the hardcoded HSV->RGB conversion is ugly here.. but it looks
like using ObjC message sends for each pixel of an image is
prohibitively slow. I agree in the long run we want real color space
handling (ICC profiles etc.) and I started implementing this in Opal
over the summer... which I'll talk more about in the report I'm
working on :-). So I would say we should live with the ugly code for
now because of the big usability improvement, and maybe in the future
delegate the conversion to Opal or a colorspace handling part of
GNUstep.

Regarding the mouse code, I didn't have a good reason to change it
other than it seemed more complex than it needed to be. But I have no
problem with changing it back if you want.

Cheers,
Eric

On Tue, Aug 24, 2010 at 2:02 AM, Fred Kiefer <address@hidden> wrote:
> Am 24.08.2010 01:34, schrieb Eric Wasylishen:
>> Author: ericwa
>> Date: Tue Aug 24 01:34:06 2010
>> New Revision: 31195
>>
>> URL: http://svn.gna.org/viewcvs/gnustep?rev=31195&view=rev
>> Log:
>> * ColorPickers/GSWheelColorPicker.m: Rewrite to draw the HSV
>> wheel in a bitmap. This gives a pretty large performance improvement.
>> * Source/NSColorPanel.m: Set a sensible min and max size for the
>> color panel.
>>
>> Modified:
>>     libs/gui/trunk/ChangeLog
>>     libs/gui/trunk/ColorPickers/GSWheelColorPicker.m
>>     libs/gui/trunk/Source/NSColorPanel.m
>
> What I like about this patch is that it removes some of the left over
> pseudo Postscript drawing code we have in gui. It should also be a great
> speed up, although I never found this specific code to be an issue.
>
> What I don't like is that it duplicates the colour space conversion once
> more. I think we already have three implementations of that in GNUstep.
> Would it be that slow to create an NSColor object and let that do the
> conversion? You only need this conversion when regenerating the image
> and you already reduced calls to that a lot.
> In the long run I would like to see all colour conversions in GNUstep
> being handled by ICC profiles and every hard coded one will be a problem
> then.
> I am not asking you to switch over to the [NSBitmapImage
> setColor:atX:y:] method, although it is there just for this purpose.
>
> What I didn't understand is the idea of the mouse event handling change
> you made. But then the code looks fine to me.
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]