You are viewing a plain text version of this content. The canonical link for it is here.
Posted to general@xmlgraphics.apache.org by Jeremias Maerki <de...@jeremias-maerki.ch> on 2010/10/28 10:51:05 UTC
[VOTE] Merge XGC color branch into Trunk
I would like to call for a vote to merge the XML Graphics Commons color
branch [1] into the Trunk. I've got the code in production since August
and it's working just fine, especially the named colors functionality.
I've now done some cleanup work and I believe the branch is now ready to
be merged into Trunk.
Related documentation and links to further information can be found at [2].
[1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
[2] http://wiki.apache.org/xmlgraphics/ColorHandling
+1 from me.
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Web Maestro Clay <th...@gmail.com>.
Is this VOTE still running? +0 from me if so.
"My religion is simple. My religion is kindness."
- His Holiness the Dalai Lama of Tibet
On Dec 15, 2010, at 3:58 AM, Adrian Cumiskey <ad...@gmail.com> wrote:
> I've not had time to follow so +0 from me.
>
> Adrian.
>
> On 2010-12-14 5:38 PM, "Jeremias Maerki" <de...@jeremias-maerki.ch> wrote:
>
> On 14.12.2010 10:17:04 Simon Pepping wrote:
>> On Tue, Dec 14, 2010 at 09:22:20AM +0100, Simon Peppin...
> Thanks
>
>
>> Note that NamedColorTest fails, but it did that already without my
>> patch.
> Fixed: http://svn.apache.org/viewvc?rev=1049003&view=rev
>
>
> Jeremias Maerki
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: gene...
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Adrian Cumiskey <ad...@gmail.com>.
I've not had time to follow so +0 from me.
Adrian.
On 2010-12-14 5:38 PM, "Jeremias Maerki" <de...@jeremias-maerki.ch> wrote:
On 14.12.2010 10:17:04 Simon Pepping wrote:
> On Tue, Dec 14, 2010 at 09:22:20AM +0100, Simon Peppin...
Thanks
> Note that NamedColorTest fails, but it did that already without my
> patch.
Fixed: http://svn.apache.org/viewvc?rev=1049003&view=rev
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: gene...
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 14.12.2010 10:17:04 Simon Pepping wrote:
> On Tue, Dec 14, 2010 at 09:22:20AM +0100, Simon Pepping wrote:
> > These are my suggestions to you. I will not again react to this issue.
> > Please merge ASAP.
>
> On second thought, done the suggested changes.
Thanks
> Note that NamedColorTest fails, but it did that already without my
> patch.
Fixed: http://svn.apache.org/viewvc?rev=1049003&view=rev
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Tue, Dec 14, 2010 at 09:22:20AM +0100, Simon Pepping wrote:
> These are my suggestions to you. I will not again react to this issue.
> Please merge ASAP.
On second thought, done the suggested changes.
Note that NamedColorTest fails, but it did that already without my
patch.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Mon, Dec 13, 2010 at 09:33:17PM +0100, Jeremias Maerki wrote:
> On 13.12.2010 20:43:43 Simon Pepping wrote:
> > On Mon, Dec 13, 2010 at 03:54:44PM +0100, Jeremias Maerki wrote:
> > > I think I have addressed all concerns now. I've switched from using
> > > equals() to ColorUtil.isSameColor(Color, Color). That should take care
> > > of the equals contract problem. If the color branch is now in a state
> > > that everybody is happy with, I'll restart the vote.
> >
> > Don't you mean the following with the class test:
> >
> > //Consider same-ness only between colors of the same class (not subclasses)
> > //but consider a ColorWithAlternatives without alternatives to be the same as a Color.
> > Class<?> cl1 = col1.getClass();
> > if (cl1 == ColorWithAlternatives.class
> > && !((ColorWithAlternatives)col1).hasAlternativeColors()) {
> > cl1 = Color.class;
> > }
> > Class<?> cl2 = col2.getClass();
> > if (cl2 == ColorWithAlternatives.class
> > && !((ColorWithAlternatives)col2).hasAlternativeColors()) {
> > cl2 = Color.class;
> > }
> > if (cl1 != cl2) {
> > return false;
> > }
>
> That's about the same, isn't it?
Not quite. Case: col1 is ColorWithAlternatives and has no
alternatives, col2 is some other subclass of Color. In your
formulation skipClassTest is true, and further tests are applied; in
mine false is returned.
I feel that my formulation does straightforwardly what the comment
says it is doing: a ColorWithAlternatives without alternatives is
considered equivalent to a Color. In your formulation, there is a
complex condition, of which it is hard to see what exactly is tested:
!(col1.getClass() == ColorWithAlternatives.class
&& !((ColorWithAlternatives)col1).hasAlternativeColors())
&&
!(col2.getClass() == ColorWithAlternatives.class
&& !((ColorWithAlternatives)col2).hasAlternativeColors())
&&
(col1.getClass() != col2.getClass())
This kind of hard to understand tests make modifying of the code, when
required, difficult. I am just trying to be helpful to you and to
later developers. Feel free to ignore my suggestion.
> > Even so, subclasses of ColorWithAlternatives may come out
> > unexpectedly: An object of ColorWithAlternatives with no alternative
> > colors, and a Color object will be equal. In contrast, an object of a
> > Subclass of ColorWithAlternatives with no alternative colors, and a
> > Color object will not be equal. Why is that so?
>
> Because I know that a ColorWithAlternatives without actual alternatives
> will behave exactly the same way as the corresponding Color object. But
> I can't say the same about some subclass of ColorWithAlternatives. Maybe
> such a subclass will add functionality that was not foreseen. In such a
> case, isSameColor() will have to be updated. I guess I should just have
> skipped this test to avoid more discussions, just considering the
> objects of two different classes different. After all, this is just
> about state checking and who cares about the rarest of rare cases where
> two such color objects follow each other?
ColorWithAlternatives is bound by the behaviour of Color.equals.
Similarly, future subclasses of ColorWithAlternatives are bound by the
behaviour of their superclass.
I feel strongly about this one. Unexpected behaviour for future
subclassing is what triggered this discussion.
> > The part where alternative colors are compared, could well be a method
> > of ColorWithAlternatives, putting the logic in the right class.
>
> Do you feel strongly about that? I don't see the advantage, especially
> since that would add a public method only used by ColorUtil.
No.
These are my suggestions to you. I will not again react to this issue.
Please merge ASAP.
> Jeremias Maerki
>
> PS: anyone want to write a master thesis on this?
Sorry for being more theoretically minded. I appreciate your numerous
contributions to FOP.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 13.12.2010 20:43:43 Simon Pepping wrote:
> On Mon, Dec 13, 2010 at 03:54:44PM +0100, Jeremias Maerki wrote:
> > I think I have addressed all concerns now. I've switched from using
> > equals() to ColorUtil.isSameColor(Color, Color). That should take care
> > of the equals contract problem. If the color branch is now in a state
> > that everybody is happy with, I'll restart the vote.
>
> Don't you mean the following with the class test:
>
> //Consider same-ness only between colors of the same class (not subclasses)
> //but consider a ColorWithAlternatives without alternatives to be the same as a Color.
> Class<?> cl1 = col1.getClass();
> if (cl1 == ColorWithAlternatives.class
> && !((ColorWithAlternatives)col1).hasAlternativeColors()) {
> cl1 = Color.class;
> }
> Class<?> cl2 = col2.getClass();
> if (cl2 == ColorWithAlternatives.class
> && !((ColorWithAlternatives)col2).hasAlternativeColors()) {
> cl2 = Color.class;
> }
> if (cl1 != cl2) {
> return false;
> }
That's about the same, isn't it?
> Even so, subclasses of ColorWithAlternatives may come out
> unexpectedly: An object of ColorWithAlternatives with no alternative
> colors, and a Color object will be equal. In contrast, an object of a
> Subclass of ColorWithAlternatives with no alternative colors, and a
> Color object will not be equal. Why is that so?
Because I know that a ColorWithAlternatives without actual alternatives
will behave exactly the same way as the corresponding Color object. But
I can't say the same about some subclass of ColorWithAlternatives. Maybe
such a subclass will add functionality that was not foreseen. In such a
case, isSameColor() will have to be updated. I guess I should just have
skipped this test to avoid more discussions, just considering the
objects of two different classes different. After all, this is just
about state checking and who cares about the rarest of rare cases where
two such color objects follow each other?
> The part where alternative colors are compared, could well be a method
> of ColorWithAlternatives, putting the logic in the right class.
Do you feel strongly about that? I don't see the advantage, especially
since that would add a public method only used by ColorUtil.
Jeremias Maerki
PS: anyone want to write a master thesis on this?
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Tue, Dec 21, 2010 at 10:01:53AM +0100, Jeremias Maerki wrote:
> The color branch of Apache XML Graphics Commons is now merged into the
> trunk.
Great work. Thanks. Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
The color branch of Apache XML Graphics Commons is now merged into the
trunk.
On 20.12.2010 09:39:00 Jeremias Maerki wrote:
> Hi Helder,
>
> I haven't restarted it. Probably doesn't make sense and would only be a
> formality since I haven't got any indication that not all objections
> were sufficiently addressed. And there were no -1s in the first place.
> Anyway, I plan to do the merge tomorrow or on Wednesday and then move on
> to prepare Batik and FOP for a merge.
>
> On 19.12.2010 18:54:46 Helder Magalhães wrote:
> > Hi everyone,
> >
> > > If the color branch is now in a state
> > > that everybody is happy with, I'll restart the vote.
> >
> > Guess I've missed that one. Wasn't clear (and still isn't) that a new
> > vote round was triggered but, although I haven't followed the last
> > changes, the concept and new features seem appealing to me as
> > previously.
> >
> > Again, +1 from me and congratulations for the effort, Jeremias! ;-)
> >
> > Cheers,
> > Helder
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Hi Helder,
I haven't restarted it. Probably doesn't make sense and would only be a
formality since I haven't got any indication that not all objections
were sufficiently addressed. And there were no -1s in the first place.
Anyway, I plan to do the merge tomorrow or on Wednesday and then move on
to prepare Batik and FOP for a merge.
On 19.12.2010 18:54:46 Helder Magalhães wrote:
> Hi everyone,
>
> > If the color branch is now in a state
> > that everybody is happy with, I'll restart the vote.
>
> Guess I've missed that one. Wasn't clear (and still isn't) that a new
> vote round was triggered but, although I haven't followed the last
> changes, the concept and new features seem appealing to me as
> previously.
>
> Again, +1 from me and congratulations for the effort, Jeremias! ;-)
>
> Cheers,
> Helder
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Mon, Dec 13, 2010 at 03:54:44PM +0100, Jeremias Maerki wrote:
> I think I have addressed all concerns now. I've switched from using
> equals() to ColorUtil.isSameColor(Color, Color). That should take care
> of the equals contract problem. If the color branch is now in a state
> that everybody is happy with, I'll restart the vote.
Don't you mean the following with the class test:
//Consider same-ness only between colors of the same class (not subclasses)
//but consider a ColorWithAlternatives without alternatives to be the same as a Color.
Class<?> cl1 = col1.getClass();
if (cl1 == ColorWithAlternatives.class
&& !((ColorWithAlternatives)col1).hasAlternativeColors()) {
cl1 = Color.class;
}
Class<?> cl2 = col2.getClass();
if (cl2 == ColorWithAlternatives.class
&& !((ColorWithAlternatives)col2).hasAlternativeColors()) {
cl2 = Color.class;
}
if (cl1 != cl2) {
return false;
}
Even so, subclasses of ColorWithAlternatives may come out
unexpectedly: An object of ColorWithAlternatives with no alternative
colors, and a Color object will be equal. In contrast, an object of a
Subclass of ColorWithAlternatives with no alternative colors, and a
Color object will not be equal. Why is that so?
The part where alternative colors are compared, could well be a method
of ColorWithAlternatives, putting the logic in the right class.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Helder Magalhães <he...@gmail.com>.
Hi everyone,
> If the color branch is now in a state
> that everybody is happy with, I'll restart the vote.
Guess I've missed that one. Wasn't clear (and still isn't) that a new
vote round was triggered but, although I haven't followed the last
changes, the concept and new features seem appealing to me as
previously.
Again, +1 from me and congratulations for the effort, Jeremias! ;-)
Cheers,
Helder
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
I think I have addressed all concerns now. I've switched from using
equals() to ColorUtil.isSameColor(Color, Color). That should take care
of the equals contract problem. If the color branch is now in a state
that everybody is happy with, I'll restart the vote.
On 03.11.2010 08:44:41 Jeremias Maerki wrote:
> On 01.11.2010 21:29:14 Vincent Hennebert wrote:
> > A few issues spotted after a quick review:
> > • there’s a non-ascii character (‘°’) in CIELabColorSpace.java
>
> fixed
>
> > • ColorExtTest should be renamed into ColorWithAlternativesTest
>
> fixed
>
> > • PSGenerator:
> > • establishColorFromColor always returns true
> > • therefore, the call to establishDeviceRBG in convertColorToPS will
> > never be made and is unnecessary
>
> Yep, something's off there. Will look into it.
>
> > • ICCColorSpaceExt
> > • the name of the class is not informative. Better use
> > ICCColorSpaceWithIntent or something like that
>
> This class comes from Batik. Will have to check what I can do here. But
> it is a reminder that I've actually been breaking
> backwards-compatibility of Batik's SVGColorProfileElementBridge.
>
> > • unimplemented methods should really throw an
> > UnsupportedOperationException or be removed altogether. Their
> > behaviour is likely to change once they are properly implemented,
> > introducing regressions. Putting a comment in the Javadoc is not
> > enough, especially since those methods can silently be called by
> > intendedToRGB
>
> I guess it makes sense to make the methods private as long as they are
> not used. Anyway, throwing UnsupportedOperationException doesn't work in
> this case. Batik or FOP would crash whenever a color profile is
> encountered that has a rendering intent that is not implemented. The
> implemented way ensures that the user gets at least an approximate
> result.
>
> > • there are missing svn properties on GrayScaleColorConverter and
> > ColorWithAlternatives
>
> ...which you could have easily fixed yourself in the time it took you to
> write this down. Fixed.
>
> > More importantly, I keep thinking that there’s a design flaw in
> > ColorWithAlternatives. Its equals method breaks the contract defined on
> > Object.equals since it’s not symmetric. This is bound to cause
> > hard-to-track issues in client code. Also, the way equals is implemented
> > will make it systematically return false if an instance of
> > ColorWithAlternatives is being compared with an instance of a sub-class,
> > which may not be the desirable result.
> >
> > Either ColorWithAlternatives is not a Color and therefore should not
> > extend the Color class; or its equals method should be changed to follow
> > the contract and the comparison of ColorWithAlternatives instances
> > should be implemented differently.
>
> Haven't I already brought this up and documented on the Wiki?
> http://wiki.apache.org/xmlgraphics/ColorHandling
>
> I guess I'll have to revisit the decisions from back then if two people
> are not happy with the current approach.
>
> > I don’t have the time nor the energy to submit a counter-proposal.
> > Therefore I’ll stay out of the way and vote -0.
> >
> > Vincent
> >
> >
> > On 28/10/10 09:51, Jeremias Maerki wrote:
> > > I would like to call for a vote to merge the XML Graphics Commons color
> > > branch [1] into the Trunk. I've got the code in production since August
> > > and it's working just fine, especially the named colors functionality.
> > > I've now done some cleanup work and I believe the branch is now ready to
> > > be merged into Trunk.
> > >
> > > Related documentation and links to further information can be found at [2].
> > >
> > >
> > > [1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
> > > [2] http://wiki.apache.org/xmlgraphics/ColorHandling
> > >
> > >
> > > +1 from me.
> > >
> > > Jeremias Maerki
>
>
>
> Jeremias Maerki
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
> For additional commands, e-mail: general-help@xmlgraphics.apache.org
>
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Vincent Hennebert <vh...@gmail.com>.
On 03/11/10 07:44, Jeremias Maerki wrote:
> On 01.11.2010 21:29:14 Vincent Hennebert wrote:
<snip/>
>> • unimplemented methods should really throw an
>> UnsupportedOperationException or be removed altogether. Their
>> behaviour is likely to change once they are properly implemented,
>> introducing regressions. Putting a comment in the Javadoc is not
>> enough, especially since those methods can silently be called by
>> intendedToRGB
>
> I guess it makes sense to make the methods private as long as they are
> not used. Anyway, throwing UnsupportedOperationException doesn't work in
> this case. Batik or FOP would crash whenever a color profile is
> encountered that has a rendering intent that is not implemented. The
> implemented way ensures that the user gets at least an approximate
> result.
Whether an approximate result is preferable to an exception or not is
a ‘business’ decision that I can’t make because I don’t know the
underlying field well. What is the user likely to prefer? How acceptable
is the fallback? If throwing an exception is impractical, is a warning
desirable? Or is it likely to produce too much noise?
Making the methods private seems to make sense indeed.
<snip/>
Vincent
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Wed, Nov 03, 2010 at 08:44:41AM +0100, Jeremias Maerki wrote:
> On 01.11.2010 21:29:14 Vincent Hennebert wrote:
>
> > More importantly, I keep thinking that there’s a design flaw in
> > ColorWithAlternatives. Its equals method breaks the contract defined on
> > Object.equals since it’s not symmetric. This is bound to cause
> > hard-to-track issues in client code. Also, the way equals is implemented
> > will make it systematically return false if an instance of
> > ColorWithAlternatives is being compared with an instance of a sub-class,
> > which may not be the desirable result.
> >
> > Either ColorWithAlternatives is not a Color and therefore should not
> > extend the Color class; or its equals method should be changed to follow
> > the contract and the comparison of ColorWithAlternatives instances
> > should be implemented differently.
>
> Haven't I already brought this up and documented on the Wiki?
> http://wiki.apache.org/xmlgraphics/ColorHandling
>
> I guess I'll have to revisit the decisions from back then if two people
> are not happy with the current approach.
Indeed, you did. Sorry for missing that. Partly inspired by the recent
discussion about findbugs, I would prefer avoiding violations of OO
contracts.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 01.11.2010 21:29:14 Vincent Hennebert wrote:
> A few issues spotted after a quick review:
> • there’s a non-ascii character (‘°’) in CIELabColorSpace.java
fixed
> • ColorExtTest should be renamed into ColorWithAlternativesTest
fixed
> • PSGenerator:
> • establishColorFromColor always returns true
> • therefore, the call to establishDeviceRBG in convertColorToPS will
> never be made and is unnecessary
Yep, something's off there. Will look into it.
> • ICCColorSpaceExt
> • the name of the class is not informative. Better use
> ICCColorSpaceWithIntent or something like that
This class comes from Batik. Will have to check what I can do here. But
it is a reminder that I've actually been breaking
backwards-compatibility of Batik's SVGColorProfileElementBridge.
> • unimplemented methods should really throw an
> UnsupportedOperationException or be removed altogether. Their
> behaviour is likely to change once they are properly implemented,
> introducing regressions. Putting a comment in the Javadoc is not
> enough, especially since those methods can silently be called by
> intendedToRGB
I guess it makes sense to make the methods private as long as they are
not used. Anyway, throwing UnsupportedOperationException doesn't work in
this case. Batik or FOP would crash whenever a color profile is
encountered that has a rendering intent that is not implemented. The
implemented way ensures that the user gets at least an approximate
result.
> • there are missing svn properties on GrayScaleColorConverter and
> ColorWithAlternatives
...which you could have easily fixed yourself in the time it took you to
write this down. Fixed.
> More importantly, I keep thinking that there’s a design flaw in
> ColorWithAlternatives. Its equals method breaks the contract defined on
> Object.equals since it’s not symmetric. This is bound to cause
> hard-to-track issues in client code. Also, the way equals is implemented
> will make it systematically return false if an instance of
> ColorWithAlternatives is being compared with an instance of a sub-class,
> which may not be the desirable result.
>
> Either ColorWithAlternatives is not a Color and therefore should not
> extend the Color class; or its equals method should be changed to follow
> the contract and the comparison of ColorWithAlternatives instances
> should be implemented differently.
Haven't I already brought this up and documented on the Wiki?
http://wiki.apache.org/xmlgraphics/ColorHandling
I guess I'll have to revisit the decisions from back then if two people
are not happy with the current approach.
> I don’t have the time nor the energy to submit a counter-proposal.
> Therefore I’ll stay out of the way and vote -0.
>
> Vincent
>
>
> On 28/10/10 09:51, Jeremias Maerki wrote:
> > I would like to call for a vote to merge the XML Graphics Commons color
> > branch [1] into the Trunk. I've got the code in production since August
> > and it's working just fine, especially the named colors functionality.
> > I've now done some cleanup work and I believe the branch is now ready to
> > be merged into Trunk.
> >
> > Related documentation and links to further information can be found at [2].
> >
> >
> > [1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
> > [2] http://wiki.apache.org/xmlgraphics/ColorHandling
> >
> >
> > +1 from me.
> >
> > Jeremias Maerki
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Tue, Nov 02, 2010 at 09:17:35PM +0100, Simon Pepping wrote:
> On Mon, Nov 01, 2010 at 08:29:14PM +0000, Vincent Hennebert wrote:
>
> > Either ColorWithAlternatives is not a Color and therefore should not
> > extend the Color class; or its equals method should be changed to follow
> > the contract and the comparison of ColorWithAlternatives instances
> > should be implemented differently.
>
> Would findbugs find this problem?
It does:
equals method overrides equals in superclass and may not be symmetric
This class defines an equals method that overrides an equals method in
a superclass. Both equals methods methods use instanceof in the
determination of whether two objects are equal. This is fraught with
peril, since it is important that the equals method is symmetrical (in
other words, a.equals(b) == b.equals(a)). If B is a subtype of A, and
A's equals method checks that the argument is an instanceof A, and B's
equals method checks that the argument is an instanceof B, it is quite
likely that the equivalence relation defined by these methods is not
symmetric.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Mon, Nov 01, 2010 at 08:29:14PM +0000, Vincent Hennebert wrote:
> More importantly, I keep thinking that there’s a design flaw in
> ColorWithAlternatives. Its equals method breaks the contract defined on
> Object.equals since it’s not symmetric. This is bound to cause
That would mean: ColorWithAlternatives a, Color b; a.equals(b) should
return the same as Color.equals if b is not of type
ColorWithAlternatives or a subclass. Only if b is of type
ColorWithAlternatives or a subclass, can ColorWithAlternatives.equals
apply its own logic.
How can symmetry be assured if the type of b is another subclass of
Color which implements its own equals? If the other class applies the
same logic (SubclassOfColor a, Color b; a.equals(b) should return the
same as Color.equals if b is not of type SubclassOfColor or a
subclass), it is guaranteed.
> hard-to-track issues in client code. Also, the way equals is implemented
> will make it systematically return false if an instance of
> ColorWithAlternatives is being compared with an instance of a sub-class,
> which may not be the desirable result.
Would it be better to use getClass().isInstance(obj), which would be
true for all objects which can be cast to ColorWithAlternatives?
> Either ColorWithAlternatives is not a Color and therefore should not
> extend the Color class; or its equals method should be changed to follow
> the contract and the comparison of ColorWithAlternatives instances
> should be implemented differently.
Would findbugs find this problem?
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Vincent Hennebert <vh...@gmail.com>.
A few issues spotted after a quick review:
• there’s a non-ascii character (‘°’) in CIELabColorSpace.java
• ColorExtTest should be renamed into ColorWithAlternativesTest
• PSGenerator:
• establishColorFromColor always returns true
• therefore, the call to establishDeviceRBG in convertColorToPS will
never be made and is unnecessary
• ICCColorSpaceExt
• the name of the class is not informative. Better use
ICCColorSpaceWithIntent or something like that
• unimplemented methods should really throw an
UnsupportedOperationException or be removed altogether. Their
behaviour is likely to change once they are properly implemented,
introducing regressions. Putting a comment in the Javadoc is not
enough, especially since those methods can silently be called by
intendedToRGB
• there are missing svn properties on GrayScaleColorConverter and
ColorWithAlternatives
More importantly, I keep thinking that there’s a design flaw in
ColorWithAlternatives. Its equals method breaks the contract defined on
Object.equals since it’s not symmetric. This is bound to cause
hard-to-track issues in client code. Also, the way equals is implemented
will make it systematically return false if an instance of
ColorWithAlternatives is being compared with an instance of a sub-class,
which may not be the desirable result.
Either ColorWithAlternatives is not a Color and therefore should not
extend the Color class; or its equals method should be changed to follow
the contract and the comparison of ColorWithAlternatives instances
should be implemented differently.
I don’t have the time nor the energy to submit a counter-proposal.
Therefore I’ll stay out of the way and vote -0.
Vincent
On 28/10/10 09:51, Jeremias Maerki wrote:
> I would like to call for a vote to merge the XML Graphics Commons color
> branch [1] into the Trunk. I've got the code in production since August
> and it's working just fine, especially the named colors functionality.
> I've now done some cleanup work and I believe the branch is now ready to
> be merged into Trunk.
>
> Related documentation and links to further information can be found at [2].
>
>
> [1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
> [2] http://wiki.apache.org/xmlgraphics/ColorHandling
>
>
> +1 from me.
>
> Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Helder Magalhães <he...@gmail.com>.
Hi everyone,
> I would like to call for a vote to merge the XML Graphics Commons color
> branch [1] into the Trunk. I've got the code in production since August
> and it's working just fine, especially the named colors functionality.
> I've now done some cleanup work and I believe the branch is now ready to
> be merged into Trunk.
Great stuff, Jeremias! :-)
> Related documentation and links to further information can be found at [2].
>
>
> [1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
> [2] http://wiki.apache.org/xmlgraphics/ColorHandling
I've been following the related threads in both Apache
(Batik/XGC)/W3C(www-svg) mailing lists and it seems a very interesting
feature to ship with what I'd like to call Batik 1.8 (and naturally
FOP.next as well, although I haven't been able to follow the project
much). ;-)
I didn't test the code myself but, given the high quality from your
previous contributions, I'm ready to deal with any potential issue
(build breakage, regard regression) from voting before making sure.
:-)
> +1 from me.
+1 from me as well.
Cheers,
Helder
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: [VOTE] Merge XGC color branch into Trunk
Posted by Simon Pepping <sp...@leverkruid.eu>.
+0 from me, since I have not looked into it. I do, however, trust that
your work is of the usual quality.
Simon
On Thu, Oct 28, 2010 at 10:51:05AM +0200, Jeremias Maerki wrote:
> I would like to call for a vote to merge the XML Graphics Commons color
> branch [1] into the Trunk. I've got the code in production since August
> and it's working just fine, especially the named colors functionality.
> I've now done some cleanup work and I believe the branch is now ready to
> be merged into Trunk.
>
> Related documentation and links to further information can be found at [2].
>
>
> [1] https://svn.apache.org/repos/asf/xmlgraphics/commons/branches/Temp_Color
> [2] http://wiki.apache.org/xmlgraphics/ColorHandling
>
>
> +1 from me.
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org