search

kmisztal / KiMage

Java Image Processing Library
6 stars 12 forks source link

Sharpen plugin #82

Closed mchuck closed 8 years ago

dkrawiec commented 8 years ago

As far as I remember we wanted to wrap AWT methods either with some class or lambda expression in order to avoid coupling between plugin implementations and AWT. Here we have direct invocations of AWT methods. This should be wrapped in imageprocessor-awt module and in plugins we should use the wrappers. @annadobrowolska am I correct?

annaskulimowska commented 8 years ago

Yes, there is the maven module for awt dependent code

-----Wiadomość oryginalna----- Od: "Damian Krawiec" notifications@github.com Wysłano: ‎2016-‎06-‎04 14:53 Do: "kmisztal/KiMage" KiMage@noreply.github.com DW: "annadobrowolska" anna90dobrowolska@gmail.com; "Mention" mention@noreply.github.com Temat: Re: [kmisztal/KiMage] Plugin (#82)

As far as I remember we wanted to wrap AWT methods either with some class or lambda expression in order to avoid coupling between plugin implementations and AWT. Here we have direct invocations of AWT methods. This should be wrapped in kimage-awt module and in plugins we should use the wrappers. @annadobrowolska am I correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

dkrawiec commented 8 years ago

And I see that we have redundant converter committed here. I know this story depends on #66 for converters, but we don't want these changes here. Please wait for merge of #79 to master and then merge it here.

Now I see that it has the same hash id so it shouldn't be a problem.

mchuck commented 8 years ago

There is no merged AWT BufferedImage converter nor any of the current AWT methods. Writing them is not really the purpose of this issue and may mislead the other implementations of plugins, so I decided to do the necessary thing in here, merging the converter from another fork and doing the AWT methods inside the plugin. I'm not sure if we should open an another issue and make these methods in AWT module, telling everyone else to use it, or leave it like it is. Up to you, but there is really not so much time left.

dkrawiec commented 8 years ago

I think that it was decided that if some plugin needs methods from the AWT then this is also part of the story to implement wrappers in imageprocessor-awt. Please implement it that way. Don't worry about time, we have enough.

mchuck commented 8 years ago

Ok, we will change that with @osipam.

dkrawiec commented 8 years ago

Ok, but keep in mind that you shouldn't create circular dependencies between branches. Commit it to one, then merge to master and master with another branch