After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 342860 - [patch] selective gaussian blur optimization
[patch] selective gaussian blur optimization
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
git master
Other All
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks: 141797
 
 
Reported: 2006-05-24 23:29 UTC by Loren Merritt
Modified: 2006-06-02 11:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sel_gauss optimization (17.74 KB, patch)
2006-05-24 23:35 UTC, Loren Merritt
none Details | Review
oops, fixes corruption when not using fpmath=sse2 (17.75 KB, patch)
2006-05-27 07:55 UTC, Loren Merritt
needs-work Details | Review
fixed-point part (7.51 KB, patch)
2006-05-29 19:06 UTC, Loren Merritt
none Details | Review
mmx part (10.83 KB, patch)
2006-05-29 19:07 UTC, Loren Merritt
committed Details | Review
slightly cleaned up version of fixed-point optimization (8.86 KB, patch)
2006-05-31 12:56 UTC, Sven Neumann
committed Details | Review
framework for accessing the use_cpu_accel configuration from libgimp (7.09 KB, patch)
2006-06-01 15:11 UTC, Sven Neumann
committed Details | Review

Description Loren Merritt 2006-05-24 23:29:56 UTC
This patch replaces the current floating-point implementation of selective gaussian blur with a fixed-point version (1.7x faster) and MMX (4x faster).
Comment 1 Loren Merritt 2006-05-24 23:35:21 UTC
Created attachment 66154 [details] [review]
sel_gauss optimization
Comment 2 Loren Merritt 2006-05-27 07:55:23 UTC
Created attachment 66317 [details] [review]
oops, fixes corruption when not using fpmath=sse2
Comment 3 Sven Neumann 2006-05-29 09:02:41 UTC
If we want to introduce MMX code in plug-ins, then we first need to introduce a way for plug-ins to access the cpu-accel configuration of the core. Including header files from the core is not sufficient (and not very elegant either) because it only gives you the compile-time configuration.

I suggest that you split the patch into two diffs. One that only does the fixed-point optimization and a second patch with the CPU-specific optimizations. We can then apply the first patch immidiately and delay the second patch until the necessary infrastructure has been added to libgimp.
Comment 4 Sven Neumann 2006-05-29 15:02:37 UTC
It would also be nice if we could move this code to libgimpmath.  Other plug-ins can probably also benefit from these optimizations.
Comment 5 Loren Merritt 2006-05-29 19:06:40 UTC
Created attachment 66429 [details] [review]
fixed-point part

OK, split.
Comment 6 Loren Merritt 2006-05-29 19:07:01 UTC
Created attachment 66430 [details] [review]
mmx part
Comment 7 Sven Neumann 2006-05-31 12:56:07 UTC
Created attachment 66527 [details] [review]
slightly cleaned up version of fixed-point optimization

This is a slightly cleaned up version of the fixed-point optimization. It fixes a couple of minor coding style issues and moves some variables to the scope where they are needed.  I would very much appreciate if you could review this patch and check that my changes don't introduce a speed regression in comparison to your version.

Given your OK I will commit this change even though it only seems to give a minor speedup.
Comment 8 Loren Merritt 2006-06-01 01:30:42 UTC
OK, there are no speed regressions.

I also confirm that the pixed-point patch makes less speedup on an Athlon-XP than my original measurements on an Athlon64. (mmx helps equally on both)
Comment 9 Sven Neumann 2006-06-01 10:17:49 UTC
2006-06-01  Sven Neumann  <sven@gimp.org>

	* plug-ins/common/sel_gauss.c: applied patch from Loren Merritt
	that replaces the floating-point implementation of selective
	gaussian blur with a fixed-point version (bug #342860).

I will look into adding the framework for using MMX in plug-ins. Let's see if we can still get this into the tree for the 2.4 release.
Comment 10 Sven Neumann 2006-06-01 15:11:13 UTC
Created attachment 66604 [details] [review]
framework for accessing the use_cpu_accel configuration from libgimp

With this patch plug-ins get access to the use_cpu_accel configuration. This is one half of the infrastructure needed. The other half is moving the CPU detection code to libgimpmath or libgimpbase.
Comment 11 Sven Neumann 2006-06-01 15:35:34 UTC
2006-06-01  Sven Neumann  <sven@gimp.org>

	Added basic framework for plug-ins to access the use_cpu_accel
	configuration (bug #342860):

	* app/composite/gimp-composite.[ch]: added new function
	gimp_composite_use_cpu_accel().

	* libgimpbase/gimpprotocol.[ch]: added use_cpu_accel to the config
	message.

	* app/plug-in/gimppluginmanager-call.c: pass the return value of
	gimp_composite_use_cpu_accel() for config.use_cpu_accel.

	* libgimp/gimp.[ch]: make the config value accessible by means of
	a new function gimp_use_cpu_accel().

	* libgimp/gimp.def: updated.
Comment 12 Sven Neumann 2006-06-01 17:08:01 UTC
Proposed API addition for libgimpmath (or libgimpbase?), please comment:

typedef enum
{
  GIMP_CPU_ACCEL_NONE        = 0x0,

  /* x86 accelerations */
  GIMP_CPU_ACCEL_X86_MMX     = 0x80000000,
  GIMP_CPU_ACCEL_X86_3DNOW   = 0x40000000,
  GIMP_CPU_ACCEL_X86_MMXEXT  = 0x20000000,
  GIMP_CPU_ACCEL_X86_SSE     = 0x10000000,
  GIMP_CPU_ACCEL_X86_SSE2    = 0x08000000,
  GIMP_CPU_ACCEL_X86_SSE3    = 0x02000000,

  /* powerpc accelerations */
  GIMP_CPU_ACCEL_PPC_ALTIVEC = 0x04000000
} GimpCpuAccelFlags;


/* query for cpu acceleration support to use */
GimpCpuAccelFlags  gimp_cpu_accel_get_support (void);

/* for internal use only */
void               gimp_cpu_accel_set_use     (gboolean use_cpu_accel);


The function gimp_cpu_accel_set_use() would be automatically called from libgimp with the return value of gimp_use_cpu_accel().  Plug-ins that query for CPU acceleration will get GIMP_CPU_ACCEL_NONE if CPU acceleration is disabled per user config.

When we have this functionality in libgimpmath, we can accept the proposed patch for selective gaussian blur and port it to this API. As a next step we should then move this code to libgimpmath so that it can be used from other plug-ins as well.
Comment 13 Loren Merritt 2006-06-01 23:52:59 UTC
The proposed API looks fine.
Comment 14 Sven Neumann 2006-06-02 06:39:43 UTC
Loren, in case you are interested, you could help us a lot by thinking about how to generalize the code so that it can be used from other plug-ins and maybe even the core.  We would like to add general matrix multiplication and convolution routines to libgimpmath.  There should also be functions in libgimp that wrap this functionality so that it can be conveniently used on pixel regions.  If you are interested in working on this, please open a new enhancement request for it.
Comment 15 Sven Neumann 2006-06-02 10:00:50 UTC
2006-06-02  Sven Neumann  <sven@gimp.org>

	Moved the CPU detection code to libgimpbase (see bug #342860):

	* app/base/Makefile.am
	* app/base/cpu-accel.[ch]
	* app/base/test-cpu-accel.c: removed here...

	* libgimpbase/Makefile.am
	* libgimpbase/gimpbase.h
	* libgimpbase/gimpcpuaccel.[ch]

	* libgimpbase/test-cpu-accel.c: ... and added here again with
	some API changes.

	* app/composite/Makefile.am
	* app/composite/make-installer.py: changed accordingly.

	* app/composite/gimp-composite-*-installer.c: regenerated.

	* libgimp/gimp.c (gimp_main): call gimp_set_use_cpu_accel().

	* libgimpbase/gimpbase.def: updated.

Comment 16 Sven Neumann 2006-06-02 11:37:19 UTC
2006-06-02  Sven Neumann  <sven@gimp.org>

	* app/composite/gimp-composite.c (gimp_composite_use_cpu_accel):
	need to test for GIMP_COMPOSITE_OPTION_NOEXTENSIONS.

	* libgimp/gimp.c (gimp_config): call gimp_set_use_cpu_accel() from
	here, not in gimp_main().

	* plug-ins/common/sel_gauss.c: applied patch from Loren Merritt
	that adds MMX code to boost the plug-in speed (bug #342860).

Closing this report as FIXED. We are however still interested in refactoring this code so that it can be used from other plug-ins.