Disabled MMX code to address Bug #86290, Bug #90969

* configure.in: GIMP_ENABLE_MMX => 0, disabling symbol
* app/paint-funcs/paint-funcs-mmx.h: disabled *_mmx() functions
*app/paint-funcs/paint-funcs.c: disabled update of layer_mode_funcs table
This commit is contained in:
Garry R. Osgood 2002-11-04 02:53:40 +00:00
parent 6d974f7294
commit 7836563081
4 changed files with 65 additions and 13 deletions

View File

@ -1,3 +1,18 @@
2002-11-03 Garry R. Osgood <grosgood@rcn.com>
* configure.in defined GIMP_ENABLE_MMX. Current value 0; to disable
related MMX code in app/paint-funcs/paint-funcs.c and
app/paint-funcs/paint-funcs-mmx.h
* app/paint-funcs/paint-funcs.c
* app/paint-funcs/paint-funcs-mmx.h MMX code bracketed by above
define Closes #86290 and #90969 by expediency: bugs were not in
MMX assembly, but in the revised wrapper introduced on 2001-11-19,
which does not accommodate correctly macros designed to run the
MMX code selectively. Rather than revert, discussion on bug report
thread opted to disable such 'MMX microoptimizations' for now, and
get the design correct first in pure C. See bug report and caveats
in paint-funcs-mmx.h
2002-11-02 Sven Neumann <sven@gimp.org>
* libgimpwidgets/gimpcolorarea.[ch]: simplified the code and

View File

@ -24,8 +24,35 @@
#ifndef __PAINT_FUNCS_MMX_H__
#define __PAINT_FUNCS_MMX_H__
/* FIXME: Needs a bigger overhaul. Maybe inline assembly would be better? */
/* FIXME: Needs a bigger overhaul. Maybe inline assembly would be better?
* 'bigger overhaul' indeed, but let's not talk about assembly language now (Nov-03-2002)
* This code is being disabled to address Bug #86290; point your favorite browser to
* http://bugzilla.gnome.org/show_bug.cgi?id=86290 for the reason this is being done.
* Presuming you've read the "Additional Comments" there, and really, really, think that
* Gimp ought to have this MMX code, RIGHT NOW (!) then this is what I believe you have to do:
* 1. Be aware that the MMX code is incomplete; the macro USE_MMX_PIXEL_OP_3A_1A has been
* written the way it has so that the MMX code is invoked only on region depths it knows
* how to handle. In that case, the MMX code executes and the macro performs a return to
* caller. Otherwise, it DOES NOT DO ANYTHING, and the macro ought to be followed by
* code that had better do something. Check out an archival app/paint_funcs/paint_funcs.c
* say, CVS version 1.98 (Nov 2, 2001) and study how USE_MMX_PIXEL_OP_3A_1A was used there;
* you'll discover that it must have some kind of alternate code following in case program
* flow does not return to the caller inside the macro.
* 2a. For this reason, you have to pretty much undo the separation of 'generic C' and 'mmx' code at
* at the heart of Nov-18-2001, Nov-19-2001 check-ins (See Daniel Egger's comments in
* ChangeLog). That, or replicate the 'generic C' in the various *_mmx functions implemented
* in this header file, below. Not only would that make the separation effort rather pointless, but
* the replication of functionality creates a maintenance problem.
* 2b. Alternatively, you can finish the MMX code so that it handles all cases that the generic
* C does. This is ideal, and makes sensible the separation effort, for then USE_MMX_PIXEL_OP_3A_1A
* need not have any additional implementation following. For that matter, you'd likely not
* even need the macro. You'd also be a better man than I am, Gunga Din.
* Whatever you do, proceed carefully and keep fresh batteries in your electric torch: there be monsters here.
* grosgood@rcn.com
*/
#ifdef HAVE_ASM_MMX
#if GIMP_ENABLE_MMX
#define MMX_PIXEL_OP(x) \
void \
@ -104,5 +131,7 @@ layer_lighten_only_mode_mmx (struct apply_layer_mode_struct *alms)
USE_MMX_PIXEL_OP_3A_1A(lighten);
}
#endif /* HAVE_ASM_MMX */
#endif /* __PAINT_FUNCS_MMX_H__ */
#endif /* GIMP_ENABLE_MMX */
#endif /* HAVE_ASM_MMX */
#endif /* __PAINT_FUNCS_MMX_H__ */

View File

@ -442,19 +442,26 @@ paint_funcs_setup (void)
for (i = 256; i <= 510; i++)
add_lut[i] = 255;
/* *_mmx functions are problematical. See "Additional Comments: Bug #86290"
* http://bugzilla.gnome.org/show_bug.cgi?id=86290 grosgood@rcn.com Nov-03-2002
*/
#ifdef HAVE_ASM_MMX
#if GIMP_ENABLE_MMX
if (use_mmx)
{
layer_mode_funcs[GIMP_DIFFERENCE_MODE] = layer_difference_mode_mmx;
layer_mode_funcs[GIMP_ADDITION_MODE] = layer_addition_mode_mmx;
layer_mode_funcs[GIMP_SUBTRACT_MODE] = layer_subtract_mode_mmx;
layer_mode_funcs[GIMP_OVERLAY_MODE] = layer_overlay_mode_mmx;
layer_mode_funcs[GIMP_SCREEN_MODE] = layer_screen_mode_mmx;
layer_mode_funcs[GIMP_MULTIPLY_MODE] = layer_multiply_mode_mmx;
layer_mode_funcs[GIMP_DARKEN_ONLY_MODE] = layer_darken_only_mode_mmx;
layer_mode_funcs[GIMP_LIGHTEN_ONLY_MODE] = layer_lighten_only_mode_mmx;
}
{
layer_mode_funcs[GIMP_DIFFERENCE_MODE] = layer_difference_mode_mmx;
layer_mode_funcs[GIMP_ADDITION_MODE] = layer_addition_mode_mmx;
layer_mode_funcs[GIMP_SUBTRACT_MODE] = layer_subtract_mode_mmx;
layer_mode_funcs[GIMP_OVERLAY_MODE] = layer_overlay_mode_mmx;
layer_mode_funcs[GIMP_SCREEN_MODE] = layer_screen_mode_mmx;
layer_mode_funcs[GIMP_MULTIPLY_MODE] = layer_multiply_mode_mmx;
layer_mode_funcs[GIMP_DARKEN_ONLY_MODE] = layer_darken_only_mode_mmx;
layer_mode_funcs[GIMP_LIGHTEN_ONLY_MODE] = layer_lighten_only_mode_mmx;
}
#endif /* GIMP_ENABLE_MMX */
#endif /* HAVE_ASM_MMX */
}
void

View File

@ -377,6 +377,7 @@ EOF
rm -f conftest.*
AC_DEFINE(HAVE_ASM_MMX, 1,
[Define to 1 if MMX in assembly is supported.])
AC_DEFINE(GIMP_ENABLE_MMX, 0, [Define to 1 to compile MMX code into paint-funcs. But read report for Bug 86290 first] )
have_asm_mmx=true
else
AC_MSG_RESULT(no)