There’s a new class in town, its name is ImageRaster. Pretty much you can take any jME3 Image you can think of, and edit it directly by setting or getting the pixel color (in a ColorRGBA) at an XY coordinate.
The new system supports all formats except for compressed formats (DXT, LATC, LTC), depth formats, RGB111110F and RGB9E5. We will never support manipulating compressed formats directly, since it just doesn’t make sense. The other 3 types of formats might be supported later on, but do note that jME3 doesn’t generate any of these formats normally and the depth formats are only used on the GPU unless you read them back to the CPU (which is not supported at the moment).
Here’s a usage example
[java]
Image myImage = …
ImageRaster raster = new ImageRaster(myImage);
raster.setPixel(1, 5, ColorRGBA.Green);
System.out.println( raster.getPixel(1, 5) ); // Will print [0.0, 1.0, 0.0, 1.0].
[/java]
A test has been added at jme3test.texture.TestImageRaster. It loads a jME3 image from the asset manager and then converts it to all supported image formats, then puts them on screen.
EDIT: There was a remark above about @zarch’s version of this system. It was not appropriate and therefore removed. See discussion below.
Cool thx! 
So as I expected you completely ignored the reasons why this was a bad way to do it and went off to do it your way anyway despite the fact that:
- The code is less readable and maintainable.
- The code will implement slower (and the compiler will find it harder to optimise).
- The “missing formats” could have been added to my solution in a fraction of the time needed to do yours from scratch.
- You do not support getting/setting of individual channels or of luminance (which could potentially be considerably faster for certain applications that are only interested in specific channels and does not cause ColorRGBA objects to be created all the time) although you do at least allow a store to be passed in which mitigates the memory thing.
To expand 1:
Just a hint:
[java] public ByteAlignedImageCodec(int bpp, int flags, int az, int rz, int gz, int bz, int ap, int rp, int gp, int bp) {
[/java]
That is terrible code as it makes things like:
[java] params.put(Format.Luminance16, new BitMaskImageCodec(2, FLAG_GRAY, 0, 16, 0, 0,
0, 0, 0, 0));
params.put(Format.Luminance16FAlpha16F, new BitMaskImageCodec(4, FLAG_GRAY | FLAG_F16,
16, 16, 0, 0,
16, 0, 0, 0));
[/java]
etc entirely unreadable without constantly flipping back to the method decleration and counting parameters. It is a maintainance/addition/bugfix/etc nightmare. You really need to buy and read Effective Java. Chapter 7 covers this.
To expand 3: Adding more formats (as explained in several places) was just a matter of implementing the methods for those formats.
If you had bothered to talk to me then it would have taken me half an hour to separate my code from Image and move it to ImageRaster (although considering most of the code was on the enum it was having only a negligible effect on Image objects anyway) and maybe an hour to implement the missing formats.
I tried to run your test but it wouldn’t run, said one of the formats wasn’t supported on my video hardware. Looking at the code for the test I can see that it only tests setPixel for multiple formats. There is no test of getPixel for any format other than BGR8.
Guess I wasted my time writing unit tests for my one too since you don’t care about proper testing either.
I’m getting really angry here. My time is valuable and I’ve got better things to do than throw it away writing a contribution (after having posted first to make sure I wasn’t interfering in any existing plans and only implementing it once a developer said to go ahead). To then have another developer come in and decide to throw out everything I’ve done is bad enough but to have them do it while entirely ignoring every post I made on the subject is just insulting.
Nice way to waste both my time and yours.
Oh, and since you seem to be in charge of this section of things what would you like doing with ImagePainter? Or is that getting thrown away too despite the fact that a lot of people want the functionality from it?
I guess I’ve got no choice but to convert it to use ImageRaster if I want long term JME3 compatibility anyway (wasting yet more of my time…thanks for that)…so it’s going to be floating around.
@zarch, i’m really sorry, but please calm down.
I understand how you feel, and i’ve foreseen this situation since the very beginning of the getPixel thing…
For Kirill’s defense he was on holidays when this all thing started, with no chance to talk his opinion on the matter.
When he got back you already had finished your implementation.
Sploreg looked interested but he went on holidays and i must admit that i just watched the post sporadically, and didn’t pay that much attention to it.
I should have warned you “watch out this is too much in the core for you to decide without Kirill’s opinion”
Next time I will.
This is deep into the core of JME, and those matters are usually trusted to Kirill because he made the core in the first place, and he’s the lead developer.
And…because most of the time no matter what you do he won’t be happy with it :D.
I’m afraid that’s what happened to you.
The good new is that yo can make him change his mind, and end up with a better solution for everyone. But you have to be convincing and choose your arguments well.
so to answer your points :
- I don’t find the example you mention that horrible…I’ve seen a lot worst. Maintainability and Readability are not objective matters, and I’d add that opinions on that matter fluctuate with time… and sometime a matter of taste.
Urging Kirill to learn how to properly code will lead you nowhere and will bring the conversation to an early end. but I guess that was more the anger talking than reason.
- Interesting, this could be a good argument for Kirill, and you should elaborate on this.
- you’re right about that, but what is done is done i’m afraid.
- That’s also interesting and should be elaborated too. In what concrete case is it useful to fetch only one channel?
About the ImagePainter, you should concretely demonstrate the use of it.
I’m not a great adept of generating textures in real time on the CPU.
IMO all applications of the ImagePainter could be replaced by baked textures or shader based texture generation, however it could be a very nice tool for the SDK to actually bake those textures.
Thus, its place in the core makes little sense to me.
You’ve been a valuable member of the community for quite some time, and i’m really sorry about this situation.
I hope that it won’t discourage you from making nice contributions and supporting people here like you do.
I feel you @zarch. I do not think it was a nice move to totally ignore your work.
@zarch: writing good code is never wasted time… and nobody ever promised your work would go to core. I can understand how this bothers you but getting angry is no solution, we all “wasted time” like this already. Open Source just doesn’t work like contractors work at a company. Also always remember that Kirill wasted his time in the first place to allow other people to create games in java so he has all the right in the world to decide how his code looks. No offense really, I just hope you see its pointless and not in your interest to make this a fight, you just want to set pixels on your images.
To be honest my main frustration is:
http://hub.jmonkeyengine.org/groups/graphics/forum/topic/adding-getpixel-in-image/?topic_page=4&num=15#post-185988
To which I replied with:
http://hub.jmonkeyengine.org/groups/graphics/forum/topic/adding-getpixel-in-image/?topic_page=4&num=15#post-186075
And got no reply at all.
I then tried several times to start a conversation and the only response I ever got was:
http://hub.jmonkeyengine.org/groups/contribution-depot-jme3/forum/topic/image-getpixelsetpixel-imagepainter-editing-jme3-images/#post-186519
Which was about as much of a brush-off as it is possible to be while remaining even vaguely polite.
@nehon said:
For Kirill's defense he was on holidays when this all thing started, with no chance to talk his opinion on the matter.
When he got back you already had finished your implementation.
Which would have been fine. Hell I'd have even ...happy is too strong but accepting... to have his implementation go in instead of mine if there was a good reason for it. However he couldn't even be bothered to discuss it and just blanked me which is incredibly insulting.
I'm just glad I didn't spend time working on supporting any more formats until I had waited to see how what I had so far was received.
@nehon said:
Sploreg looked interested but he went on holidays and i must admit that i just watched the post sporadically, and didn't pay that much attention to it.
I should have warned you "watch out this is too much in the core for you to decide without Kirill's opinion"
Next time I will.
That might have saved a lot of hassle...
It's especially annoying for me because I knew this was fairly close to the core and there might be existing plans etc. So I asked the question and I waited until a developer said to go ahead (and none had objected) before I did anything. As an outsider I've no idea what the internal hierarchy is and who is responsible for what so I can only go off what I'm told.
@nehon said:
This is deep into the core of JME, and those matters are usually trusted to Kirill because he made the core in the first place, and he's the lead developer.
And...because most of the time no matter what you do he won't be happy with it :D.
I'm afraid that's what happened to you.
The good new is that yo can make him change his mind, and end up with a better solution for everyone. But you have to be convincing and choose your arguments well.
so to answer your points :
1. I don't find the example you mention that horrible...I've seen a lot worst. Maintainability and Readability are not objective matters, and I'd add that opinions on that matter fluctuate with time... and sometime a matter of taste.
Urging Kirill to learn how to properly code will lead you nowhere and will bring the conversation to an early end. but I guess that was more the anger talking than reason.
Yeah. I was venting a bit of frustration. My point on maintainability stands though, any parameter list with that number of parameters (particularly when most are of the same type) is a nightmare to work with. If he'd replied to my initial post there are any number of ways to reduce/mitigate the problem that we could have discussed but I just got ignored.
@nehon said:
2. Interesting, this could be a good argument for Kirill, and you should elaborate on this.
With the original implementation getPixel on RGB would immediately delegate to the enumeration and call the relevant method. That would read the data in the correct format - even taking advantage of things like subsequent getByte being able to read the correct values without needing to seek around. This is both much easier for the compiler to optimise as the call stack is simpler and there is no branching and no map look ups and directly faster due to avoiding any unneeded seeks or bitwise operations. Without running performance tests I've no idea whether the difference is 1% or 400% (either is entirely possible) but the only thing I can be sure of is that the new approach will be at least a little slower.
I might do some performance tests later as I'm going to need to move ImagePainter over to be built on top of ImageRaster assuming performance is acceptable.
Take a simple example of RGBA - in that case you can literally do getByte 4 times, dump the value into a ColorRGBA and you are done. All the other byte-per-channel formats are also nearly as simple. In the new implementation though I've not looked in detail but it seems to read them into a local store, move them around, bitmask them, etc because the complexity needed for the other formats is still there even in this simple case.
Things I like about the new implementation:
1. Obviously it avoids the repeated similar (but not identical) code that I wasn't keen on either in my one.
2. EnumMap FTW. (int flags instead of EnumSet not so good though)
3. Splitting up into BitMask, ByteAligned etc is good and how I considered doing it when I was thinking about this approach
I don't think the new implementation is bad. In fact when I was deciding it was pretty much a toss up as to which way was better to go in the first place. My approach has the advantage of simplicity and performance, the other approach has less repeated similar code. What the new implementation is very definitely not though is better than mine - which makes the work involved in throwing away mine and rewriting it with worse performance, fewer features, and more complexity entirely baffling.
@nehon said:
3. you're right about that, but what is done is done i'm afraid.
4. That's also interesting and should be elaborated too. In what concrete case is it useful to fetch only one channel?
An obvious example is heightmaps which often use either getLuminance or getRed. Additionally a number of formats are single-channel only (for example Luminance8 and Alpha8) so if you know you are dealing with that format you can work more efficiently using that knowledge.
Androlo's biomes stuff and the terrain shader uses individual channels to control different things. Again there may be cases where you are only interested in querying one channel although I don't know enough about them to give concrete examples there.
@nehon said:
About the ImagePainter, you should concretely demonstrate the use of it.
I'm not a great adept of generating textures in real time on the CPU.
IMO all applications of the ImagePainter could be replaced by baked textures or shader based texture generation, however it could be a very nice tool for the SDK to actually bake those textures.
Thus, its place in the core makes little sense to me.
For myself I'm making a trading card game so it's used to generate the textures for the cards dynamically based on the stats of the cards. Other examples might include things like a signpost with writing on it - where you dynamically put in the correct translation or even use one signpost model/base texture and then generate the text needed for that notice and have it as text not graphical data.
Various people have been looking at editing heightmaps etc, the features could be useful for that.
It can make procedural texture generation easier (although in many cases get/set pixel is enough for that).
Digital clocks or other displays embedded in the scene - stuff where it changes occasionally but not every frame or again where it should be translated to a local language. Displays in a space ship. (Projecting nifty into a texture works but is far too slow to use for more than 1 or 2 textures at a time and massive overkill for textures that only change occasionally). Menus in a bar which are updated as the price or availability of items changes...etc ... etc
The transcoding and scaling functionality could be used to do things like convert textures to lower resolution formats (either lower width/height or lower bpp or both) for android on loading (although I agree it would be better to do that in advance when packaging the android app).
@nehon said:
You've been a valuable member of the community for quite some time, and i'm really sorry about this situation.
I hope that it won't discourage you from making nice contributions and supporting people here like you do.
Well it's a bit late for me to change from JME3 to something else without a lot of rewriting so I'm not going to go anywhere. It's certainly not going to encourage me to take the time to package up anything I write for other people to use as well though if the result is going to be that I get ignored for 2 weeks and then what I wrote thrown away.
@normen said:
@zarch: writing good code is never wasted time.. and nobody ever promised your work would go to core. I can understand how this bothers you but getting angry is no solution, we all "wasted time" like this already. Open Source just doesn't work like contractors work at a company. Also always remember that Kirill wasted his time in the first place to allow other people to create games in java so he has all the right in the world to decide how his code looks. No offense really, I just hope you see its pointless and not in your interest to make this a fight, you just want to set pixels on your images.
Code that gets used isn't wasted. Code that you learn something from is not wasted...This is neither of those.
JME3 is alive and being used by hundreds (thousands?) of people. That's a long way from wasted and I'm grateful for all the time people have spent on the engine and have tried to repay that both by helping support on the forums and by providing things for other people to use. I can respect the time and the work Kirill and everyone else has done and continues to do on the engine.
That doesn't mean I'm not going to bite back if someone kicks me in the face though. The time spent on this could have been spent contracting to earn me £, or working on getting HeroDex closer to release, or packaging the nifty popup stuff I'd been considering releasing for people, or any other useful things. My time is very finite and I never have enough hours in the day so I'm extremely protective of people other than me wasting it.
Right, I've said my piece and vented a bit. I'm quite happy to answer any technical questions and hopefully have a constructive discussion but other than that I'm going to move on. I might do some performance tests later since I'm actually quite curious to see how much difference it does make and if I do I'll post the results.
so is it when someone engages without the effects, everyone understand your angry
there was a topic about contribution, and why people contributing.
but there were nothing about not accepted “contributions”. and here is an example.
i’m sure that other devs are sometimes angry on lead dev decisions, but they need to be meek.
their advantage is that they are on their own chat or something as i good remember.
myself I am amazed at your contribution, which has not been added. and I sympathize with you.
you just need to go on.
ahh didn’t realise it went this deep. I think the least that could have been done, was have a detailed discussion between the 2 to reach a comprise and where the 2 could have worked together, or that zarch would know what stuff needed changing. Seems zarch worked very hard on this, and that he was waiting when (not if) it was being added, and this came as a big shock to him, which is understandable, I would be the same
I have no dog in this fight, but COURTESY dictates, as in all good things centered around a team, that others discuss proposed solutions and work from there. I mean, isn’t this OPEN SOFTWARE? Aren’t we supposed to all push in the same direction?
I don’t see why this needed to be rewritten from scratch. All Kirill had to do was talk to Zarch and explain to him what he wanted. I’m sure Zarch would have obliged (maybe not happily, but he would’ve).
Now, what we get is 2 bruised egos. That is not the way to do this. I’m sorry to say.
@madjack said:
I don't see why this needed to be rewritten from scratch. All Kirill had to do was talk to Zarch and explain to him what he wanted. I'm sure Zarch would have obliged (maybe not happily, but he would've).
Now, what we get is 2 bruised egos. That is not the way to do this. I'm sorry to say.
Problem is none of the devs overseeing this were available at the time. Brent answered in the second post or so and Kirill answered about at the end of the thread.. In between all the work was happening. Sorry to say but this is an impossible situation and could have been avoided by zarch by just waiting or by Kirill or Sploreg appearing with time to see whats happening and stopping it for now or taking the time to explain whats needed..
Sorry, zarch. This has happened to me before on a few different open source projects and it never gets any easier. In one case, I’d submitted a rewrite to a section of Tomcat, it was accepted and included… then I stopped watching the dev mailing list for a few months and someone (not even a committer) submitted a patch that overwrote my changes because they were not developing against head. This was accepted and my code was forever blown away. To rub salt in the wound, this person ended up being granted committer status for their changes. I was seething about that for a few years.
Now, if I can, I try to develop everything as a separate contribution when possible… and then only develop stuff I need. Then there is minimal extra work to get it polished for general distribution. If other people like it and use it, great. If it gets included in core, also great. If not then it’s no big deal because I needed it anyway.
ImagePainter is a good example of this, I think. It’s an excellent contrib project because it has limited utility for the larger population… but for those few who need it then it is the perfect fit. Anyone who has to make their own textures and is targeting android or for some other reasons doesn’t want to use Java2D and can live with the much smaller feature set, ImagePainter will be great. If it’s a convenient plug-in that they can grab with a click then even better.
@madjack said:
I have no dog in this fight, but COURTESY dictates, as in all good things centered around a team, that others discuss proposed solutions and work from there. I mean, isn't this OPEN SOFTWARE? Aren't we supposed to all push in the same direction?
I don't see why this needed to be rewritten from scratch. All Kirill had to do was talk to Zarch and explain to him what he wanted. I'm sure Zarch would have obliged (maybe not happily, but he would've).
Now, what we get is 2 bruised egos. That is not the way to do this. I'm sorry to say.
This is a nice idea... but when time is limited... this means that what actually would have happened is: nothing.
These conversations are draining in general and so it's more likely that Kirill would have just found something else to do and zarch's thread would have been left to zombie-bump for months.
Something we all sometimes forget: code _IS_ a form of conversation. The end of this story is not yet written. Someone proposed one solution, someone else proposed a different one... it just has a little more weight because now it's in core. It is not part of a stable update. A convincing argument could still change it to be some better super-hybrid.
...but we have to get past the bruised egos and deal with things on their own merit.
@normen said:
Problem is none of the devs overseeing this were available at the time. Brent answered in the second post or so and Kirill answered about at the end of the thread.. In between all the work was happening. Sorry to say but this is an impossible situation and could have been avoided by zarch by just waiting or by Kirill or Sploreg appearing with time to see whats happening and stopping it for now or taking the time to explain whats needed..
In the end the result is the same. Everyone has a fault. Kirill didn't fully explain. Splored didn't make an appearance. Zarch jumped the gun. I didn't intervene. Nehon didn't intervene, You didn't intervene.
What we have here is a classic case of miscommunication, plain and simple. (And a case of Zarch being a bit too eager, which is very understandable, but slightly misplaced.)
Now everyone needs to take a deep breath. Playing the blaming game won't help anyone, plus it's counter-productive.
I try to say there is noone to blame and not to blame somebody Also note pauls comments on the first page, they kind of collided with our answers here.
That comment about zarch was not appropriate in the first post so I removed it.
@zarch: I apologize if I offended you and didn’t give you enough info to go on. The thing with contributions, you cannot just expect them to go in, sometimes modifications will need to be made. Sometimes this is not enough, and the whole thing has to be rewritten. I think this was one of those cases, and for me it was easier to rewrite it myself rather than have somebody else do it.
It is our (jME3 developers) fault that we initially told you to go ahead and then left the thread, you were working hard on this thinking it was going to go in.
@pspeed said:
This is a nice idea... but when time is limited... this means that what actually would have happened is: nothing.
It is a nice idea and it works when everyone is on the same page. In this case we had something, not nothing. Zarch had his modifications and they were working (afaik). So instead of dialoguing with what was made, Kirill went off without much of an explanation and rewrote the whole thing.
There are no real loser in the end as the feature gets implemented and available for anyone who might need it. There's only a loss of time, which can be frustrating.
@normen said:
I try to say there is noone to blame and not to blame somebody :) Also note pauls comments on the first page, they kind of collided with our answers here.
No biggie Normen. I wasn't saying you were trying to blame someone. Just underlying the fact that it shouldn't be done.