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 <[email protected]> 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. > > _______________________________________________ Gnustep-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnustep-dev
