Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
Joao S. O. Bueno ([EMAIL PROTECTED]) wrote: I've tried Simons Patch, and it seemed very nice for me. Of course I am innoi position to word out what should and should not be commited, but from a user point of view, it is nice. There are two things I'd like to know. As you know Gimp avoids opening too big image windows when loading an image. Right now the size of the image area is restricted to 0.75 * screen dimensions. This of course is perfectly Ok. However, I'd like to know which of the two following behaviours is preferrable in case of an image being too big for the screen: a) open the image as big as possible (zoom-to-fit to a window about 0.75 * screen dimensions), this roughly is the behavior of current CVS. b) open the image in the next smaller zoom preset (which would result in image windows smaller than the 0.75 * screen dimensions, but would have nice ratios) (since CVS does not yet really have any zoom presets its hard to compare...) Also I'd like to know if the zoom steps around 100% are fine grained enough. Homogenous zooming right now is implemented with a factor of 2^(1/2) (from 100% to 200% in two steps), but 2^(1/3), 2^(1/4) would work as well (three, resp. four steps from 100% to 200%) and give finer grained steps. Opinions? Thanks, Simon -- [EMAIL PROTECTED] http://www.home.unix-ag.org/simon/ ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-user] Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
Simon Budig ([EMAIL PROTECTED]) wrote: a) open the image as big as possible (zoom-to-fit to a window about 0.75 * screen dimensions), this roughly is the behavior of current CVS. b) open the image in the next smaller zoom preset (which would result in image windows smaller than the 0.75 * screen dimensions, but would have nice ratios) (since CVS does not yet really have any zoom presets its hard to compare...) Oops, sorry I mixed that up. Right now Gimp-CVS uses the old zoom steps when opening a new image (kind of behaviour b). My patch implements a) here and I got confused with the two different GIMPs... :-) Bye, Simon -- [EMAIL PROTECTED] http://www.home.unix-ag.org/simon/ ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-user] Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Wednesday 21 January 2004 12:27, Simon Budig wrote: Joao S. O. Bueno ([EMAIL PROTECTED]) wrote: I've tried Simons Patch, and it seemed very nice for me. Of course I am innoi position to word out what should and should not be commited, but from a user point of view, it is nice. There are two things I'd like to know. As you know Gimp avoids opening too big image windows when loading an image. Right now the size of the image area is restricted to 0.75 * screen dimensions. This of course is perfectly Ok. However, I'd like to know which of the two following behaviours is preferrable in case of an image being too big for the screen: a) open the image as big as possible (zoom-to-fit to a window about 0.75 * screen dimensions), this roughly is the behavior of current CVS. b) open the image in the next smaller zoom preset (which would result in image windows smaller than the 0.75 * screen dimensions, but would have nice ratios) (since CVS does not yet really have any zoom presets its hard to compare...) Hmm... Actually, 0.75 is sometimes boring, when the whole image would fit in, say, 90% of the screen, and it shows up zoomed out. regarding your specific question, it would not be nice if the GIMP openned an image in a zoom factor that once changed could not get easily reproduced. So the answer is (b).However, if you could make it in a way that if the next bigger zoom ratio (in the 2^(1/2) steps you use) would be no larger than 80% or maybe 85% of the screen it would be the one used. On the other hand, I was not around when the choice for 75% was made, and there may be strong motives for that. Also I'd like to know if the zoom steps around 100% are fine grained enough. Homogenous zooming right now is implemented with a factor of 2^(1/2) (from 100% to 200% in two steps), but 2^(1/3), 2^(1/4) would work as well (three, resp. four steps from 100% to 200%) and give finer grained steps. Yes, it seens just ok. I would not like to have to hit '+' four times to get a image twice as large. Now let's see what others have to say. Opinions? Thanks, Simon Regards, JS -- ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-user] Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
Hi, Joao S. O. Bueno [EMAIL PROTECTED] writes: Actually, 0.75 is sometimes boring, when the whole image would fit in, say, 90% of the screen, and it shows up zoomed out. regarding your specific question, it would not be nice if the GIMP openned an image in a zoom factor that once changed could not get easily reproduced. So the answer is (b).However, if you could make it in a way that if the next bigger zoom ratio (in the 2^(1/2) steps you use) would be no larger than 80% or maybe 85% of the screen it would be the one used. On the other hand, I was not around when the choice for 75% was made, and there may be strong motives for that. IIRC, 75% was choosen rather arbitrarily and I agree that it would make sense to use 85% or even 90% instead and choose the closest sane display ratio below. Sven ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 04:13:19AM +0100, Marc A. Lehmann wrote:
On Tue, Jan 20, 2004 at 02:22:57AM +0100, Simon Budig [EMAIL PROTECTED] wrote:
other parts, and I already had enough with C guts) and is small, it
just fits in place with the old code instead of more deep changes.
True. (These break strict aliasing rules warnings however are harmless
according to Yosh.)
Just a sidenote, unless caused by a bug in the compiler, these warnings
are never harmless. They might not cause problems with current gcc,
but there is no guarentee that the code will do as expected with other
compilers or future versions of gcc, unless one uses -fno-strict-aliasing,
which can be a major performance problem in some cases.
Well, the bulk of the code in gimp that causes warnings is stuff like:
void foo (void **p);
void bar (void)
{
int *i;
foo ((void **) i);
}
While it does break the letter of the law wrt aliasing rules, are there any
assumptions that the compiler can legally make that would cause problems?
-Yosh
___
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
[Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 01:24:15AM -0800, Manish Singh wrote:
On Tue, Jan 20, 2004 at 04:13:19AM +0100, Marc A. Lehmann wrote:
On Tue, Jan 20, 2004 at 02:22:57AM +0100, Simon Budig [EMAIL PROTECTED] wrote:
other parts, and I already had enough with C guts) and is small, it
just fits in place with the old code instead of more deep changes.
True. (These break strict aliasing rules warnings however are harmless
according to Yosh.)
Just a sidenote, unless caused by a bug in the compiler, these warnings
are never harmless. They might not cause problems with current gcc,
but there is no guarentee that the code will do as expected with other
compilers or future versions of gcc, unless one uses -fno-strict-aliasing,
which can be a major performance problem in some cases.
Well, the bulk of the code in gimp that causes warnings is stuff like:
void foo (void **p);
void bar (void)
{
int *i;
foo ((void **) i);
}
does this count the things that you cannot do? it would be very silly
to tie a process up with this sort of thing. this list is infinite.
While it does break the letter of the law wrt aliasing rules, are there any
assumptions that the compiler can legally make that would cause problems?
does gcc do this? count up things that it is not allowed to do? will
the compiler need to be rewritten?
carol
___
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
I've tried Simons Patch, and it seemed very nice for me. Of course I am innoi position to word out what should and should not be commited, but from a user point of view, it is nice. Unfortunately I could not check GSR's patch because of compiling issues. Regards, JS -- On Monday 19 January 2004 23:22, Simon Budig wrote: (...) Unfortunately these two patches basically exclude each other and it will come to no surprise to you, that I'd prefer to see my own patch in 2.0. However, because it is so intrusive I won't commit it until somebody read my patch and gives me the OK to do it. Bye, Simon ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 01:24:15AM -0800, Manish Singh [EMAIL PROTECTED] wrote:
Well, the bulk of the code in gimp that causes warnings is stuff like:
void foo (void **p);
void bar (void)
{
int *i;
foo ((void **) i);
}
While it does break the letter of the law wrt aliasing rules, are there any
assumptions that the compiler can legally make that would cause problems?
Well, troubling to me would be the fact that a int is not the same as
a void *, so this very example is a bit strange and could likely cause
problems on 64-bit (or else) architectures, but that is, of course, not
the main point.
I don't know the real case this is based on, but I'd wonder what this code
is supposed to do then. Please note that the warning will not happen when
you cast to a void *, since the pointers might alias then.
Legally, the compiler could cache the contents of i in a register before
and after the call, because foo is not allowed to change it's value. (And
I might guess that foo will not do that, but it's equally hard to see this
as a compiler as it is for a human, so the warning is IMnsO justified).
It's unlikely that this will happen with gcc = 3.5, which is, IMHO,
a viable platform to tie oneself to, but there are no guarentees that
this won't happen in more complicated cases, with other (less or more
intelligent) compilers or with future gcc versions.
It's hard to judge from his example, but right now it's difficult for
me to imagine a valid use for the above that couldn't easily be written
correctly.
To repeat it: I am quite certain that the above example will simply work
for quite some time in the future, because of some gcc assumptions about
pointers that are difficult to change but make no good otimized code.
Perl does similar pointer castings and has opted the safe way by simply
using -fno-strict-aliasing to compile itself at all times, after being
bitten once.
That might be easier then going through all the code and fixing it, and
will of course silence the warning. Right now, it doesn't make much of a
difference in generated code with gcc, as it is not very good at taking
advantage of (no-) aliasing yet, but this is a hot area in gcc, since
exploiting aliasing rules allow a great deal of optimizations in typical
numerical code.
--
-==- |
==-- _ |
---==---(_)__ __ __ Marc Lehmann +--
--==---/ / _ \/ // /\ \/ / [EMAIL PROTECTED] |e|
-=/_/_//_/\_,_/ /_/\_\ XX11-RIPE --+
The choice of a GNU generation |
|
___
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 02:51:17PM +0100, Marc A. Lehmann wrote:
On Tue, Jan 20, 2004 at 01:24:15AM -0800, Manish Singh [EMAIL PROTECTED] wrote:
Well, the bulk of the code in gimp that causes warnings is stuff like:
void foo (void **p);
void bar (void)
{
int *i;
foo ((void **) i);
}
While it does break the letter of the law wrt aliasing rules, are there any
assumptions that the compiler can legally make that would cause problems?
Well, troubling to me would be the fact that a int is not the same as
a void *, so this very example is a bit strange and could likely cause
problems on 64-bit (or else) architectures, but that is, of course, not
the main point.
You misread. To explain further:
void set_null (void **p)
{
*p = NULL;
}
void foo (void)
{
int *i;
set_null ((void **) i);
}
Which basically sets i = NULL. I don't see how this can be a problem on a
64-bit architectures, since pointers to all types are the same size.
gimp actually only does this with external functions, like:
voidg_object_add_weak_pointer (GObject*object,
gpointer *weak_pointer_location);
or
gboolean g_module_symbol(GModule *module,
const gchar *symbol_name,
gpointer *symbol);
Perhaps the API should be fixed, I dunno.
I don't know the real case this is based on, but I'd wonder what this code
is supposed to do then. Please note that the warning will not happen when
you cast to a void *, since the pointers might alias then.
Sure, I know ways of working it around it. My preferred fix is not to
cast to a void *, but use a temporary void * and assign back, since that
adds some type safety instead of casting.
Legally, the compiler could cache the contents of i in a register before
and after the call, because foo is not allowed to change it's value. (And
I might guess that foo will not do that, but it's equally hard to see this
as a compiler as it is for a human, so the warning is IMnsO justified).
Ah, that answers my question, thanks.
It's unlikely that this will happen with gcc = 3.5, which is, IMHO,
a viable platform to tie oneself to, but there are no guarentees that
this won't happen in more complicated cases, with other (less or more
intelligent) compilers or with future gcc versions.
It's hard to judge from his example, but right now it's difficult for
me to imagine a valid use for the above that couldn't easily be written
correctly.
To repeat it: I am quite certain that the above example will simply work
for quite some time in the future, because of some gcc assumptions about
pointers that are difficult to change but make no good otimized code.
Perl does similar pointer castings and has opted the safe way by simply
using -fno-strict-aliasing to compile itself at all times, after being
bitten once.
That might be easier then going through all the code and fixing it, and
will of course silence the warning. Right now, it doesn't make much of a
difference in generated code with gcc, as it is not very good at taking
advantage of (no-) aliasing yet, but this is a hot area in gcc, since
exploiting aliasing rules allow a great deal of optimizations in typical
numerical code.
Well, one problem is that flagging every violation like this makes people
tend to ignore the warning, which means real problems which affect gcc's
alias analysis *now* are likely to be overlooked, or heavy handed solutions
like -fno-strict-aliasing put in. It'd be much nicer if this warning didn't
trigger for function parameters (except with -pedantic) until gcc was closer
to doing the alias analysis that would manifest a problem.
As it stands, right now the warning mainly confuses people. It doesn't help
that the aliasing rules aren't spelled out anywhere in the gcc docs. I was
actually skeptical that gcc might be buggy in this regard, until I dug up
the appropriate bits of the standard that say what's going on.
-Yosh
___
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
[restricting this to gimp-devel, since this is purely technical stuff] GSR - FR ([EMAIL PROTECTED]) wrote: [EMAIL PROTECTED] (2004-01-19 at 1524.44 +0100): [technical discussion :)] I think I already explained why I prefer the set of ratios based on the idea of homogenous zooming. So the rest of this Mail focuses on the technical issues of your patch. The last patch I sent does homogenous zooming, has no more (known) floating issues (I am not gonna bet about floats, what is more, not gonna bet about Gimp either, I saw some warnings about aliasing in other parts, and I already had enough with C guts) and is small, it just fits in place with the old code instead of more deep changes. True. (These break strict aliasing rules warnings however are harmless according to Yosh.) [... code] As you can see it compares floats with = and = and so avoids tests for real equalness. The little jump done by multiplying by 1.1 (which is a bit arbitrary chosen, but should be smaller than the factors between the presets) makes it even more robust IMHO. So in the end both do a list of presets. Main difference is that mine is simetrical (that can be good or bad) and uses a typical search system. BTW, the CLAMP should be 255 or 256? Well, I'd still prefer real homogenous zooming, but can see the issues you and some other people seem to have with it. The stuff that is currently in CVS (that tries to do homogenous zooming and forces the fractions towards small numerator/denominator - sane fractions) would be incredibly hard to tweak to get the desired behaviour (without presets that is). The Code snippet is part of an API change, that allows to specify floats as the magnification factor consistently. The 255 basically was imposed, because the earlier code encoded the fractions in a single integer and is limited to that value. When I changed that to floats, that limit became meaningless and I choose 256 over 255, since this is the logical successor in the sequence. [...] I hope you can accept this as a technical criticism of your patch, it might solve your floating point problems with a different approach. It also should work with a different set of presets. Problems solved... what do you mean with different set? Both can be adjusted, that was one of the differences from my first to second version, the table changed. I mentioned this, because the main motivation for you to start with the patch seemed to be that you were unhappy about the way the current CVS code is stepping, which is based on homogenous zooming with less nice fractions. I wanted to express that - contrary to the current CVS code - tweaks to the presets are possible. Ok. We now have your plug-n-play patch to solve the zooming issue and we have a patch from me, that is a lot more invasive, needs some more attention for some things, but also fixes more things (API, Zoom-dialog). Unfortunately these two patches basically exclude each other and it will come to no surprise to you, that I'd prefer to see my own patch in 2.0. However, because it is so intrusive I won't commit it until somebody read my patch and gives me the OK to do it. Bye, Simon -- [EMAIL PROTECTED] http://www.home.unix-ag.org/simon/ ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 02:22:57AM +0100, Simon Budig wrote: [restricting this to gimp-devel, since this is purely technical stuff] GSR - FR ([EMAIL PROTECTED]) wrote: [EMAIL PROTECTED] (2004-01-19 at 1524.44 +0100): [technical discussion :)] I think I already explained why I prefer the set of ratios based on the idea of homogenous zooming. So the rest of this Mail focuses on the technical issues of your patch. The last patch I sent does homogenous zooming, has no more (known) floating issues (I am not gonna bet about floats, what is more, not gonna bet about Gimp either, I saw some warnings about aliasing in other parts, and I already had enough with C guts) and is small, it just fits in place with the old code instead of more deep changes. True. (These break strict aliasing rules warnings however are harmless according to Yosh.) Well, not all are harmless. Just that a lot of them are spurious, but some of them do signify real problems. It's a mess. -Yosh ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Re: Re: Re: Alternative zoom algorithm
On Tue, Jan 20, 2004 at 02:22:57AM +0100, Simon Budig [EMAIL PROTECTED] wrote: other parts, and I already had enough with C guts) and is small, it just fits in place with the old code instead of more deep changes. True. (These break strict aliasing rules warnings however are harmless according to Yosh.) Just a sidenote, unless caused by a bug in the compiler, these warnings are never harmless. They might not cause problems with current gcc, but there is no guarentee that the code will do as expected with other compilers or future versions of gcc, unless one uses -fno-strict-aliasing, which can be a major performance problem in some cases. -- -==- | ==-- _ | ---==---(_)__ __ __ Marc Lehmann +-- --==---/ / _ \/ // /\ \/ / [EMAIL PROTECTED] |e| -=/_/_//_/\_,_/ /_/\_\ XX11-RIPE --+ The choice of a GNU generation | | ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
[Gimp-developer] Re: Re: Re: Alternative zoom algorithm
[EMAIL PROTECTED] (2004-01-19 at 1524.44 +0100): [technical discussion :)] I think I already explained why I prefer the set of ratios based on the idea of homogenous zooming. So the rest of this Mail focuses on the technical issues of your patch. The last patch I sent does homogenous zooming, has no more (known) floating issues (I am not gonna bet about floats, what is more, not gonna bet about Gimp either, I saw some warnings about aliasing in other parts, and I already had enough with C guts) and is small, it just fits in place with the old code instead of more deep changes. [... code] As you can see it compares floats with = and = and so avoids tests for real equalness. The little jump done by multiplying by 1.1 (which is a bit arbitrary chosen, but should be smaller than the factors between the presets) makes it even more robust IMHO. So in the end both do a list of presets. Main difference is that mine is simetrical (that can be good or bad) and uses a typical search system. BTW, the CLAMP should be 255 or 256? [...] I hope you can accept this as a technical criticism of your patch, it might solve your floating point problems with a different approach. It also should work with a different set of presets. Problems solved... what do you mean with different set? Both can be adjusted, that was one of the differences from my first to second version, the table changed. GSR ___ Gimp-developer mailing list [EMAIL PROTECTED] http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer
