Image class critique

[eaolson]

I've been playing around with images, and have come up with the attached set of classes to hopefully eliminate the somewhat complicated way LabVIEW handles images loaded from disk. The "flattened pixmap" approach is a pretty clever way to handle both truecolor and indexed images in the same data structure, but it's unnecessarily complicated in my book and can lead to some problems if the image you load is not the image you expected at design time.

The transparency aspects of loading the images may not be exactly correct, which is why I haven't put this in the CR yet. It seems LabVIEW doesn't load the alpha channel exactly as it's described in the help text when the image is very small.

This is my first real foray into LVOOP, so I would appreciate any critiques or comments anyone would offer, on anything from whether it works as it should to the OOP design.

[Yair]

I'm definitely not the most appropriate person to respond to this as I haven't done any LVOOP either and I never really had to deal with indexed images, so I don't really understand the need for this.

A couple of notes I will make, however:

• It would make it easier to understand which class is which if they had different icons. As it is, when all the VIs have the same icon, you just get the feeling you're opening the same VI over and over.
  
• Did you save this on a different drive than your C drive? It looks for the 8.2 vi.lib, which means it did not save it as a logical path and I believe that's the usual reason for that.
  

[Tomi Maila]

I'd like to take a look at this but just now I don't have time. If I've not replied in a week, please send me a PM to remind me

[eaolson]

QUOTE(yen @ Sep 1 2007, 03:00 PM)

• It would make it easier to understand which class is which if they had different icons. As it is, when all the VIs have the same icon, you just get the feeling you're opening the same VI over and over.
  
• Did you save this on a different drive than your C drive? It looks for the 8.2 vi.lib, which means it did not save it as a logical path and I believe that's the usual reason for that.
  

1. Art was not one of my best classes, so I'm not so great at making icons. I'll try to come up with some way of differentiating them. My idea, though, was that you'd just use the methods from the Image class and it would dynamic dispatch to the correct child method, so you'd never need to see the icons for the subclasses.

2. No I didn't save it on a different drive and it gives me that notice, too. I'm not sure why. I actually made a source distribution in the project file I'm using to aggregate all the VIs to one folder, so I suspect it has something to do with that.

[Tomi Maila]

My first initial comment is that you are using two OpenG library VI's in your project. The functionality of both of these VIs is very simple. I'd include these VIs into the project with new names instead of requiring the class user to download and install the appropriate OpenG packages.

I took a quick look at the project and there are some issues with the roles of different classes I'd like to suggest you to modify. The reponsibilities of the classes are overlapping, which is not generally considered a good OOP design. However I still need to take a longer look before any details.

[Aristos Queue]

I'd like to suggest a change to the factory function. The factory function "Image.lvclass:Create Image.vi" is the piece that takes the input image data and outputs the correct image class.

Currently you have a case structure that splits out "24, 32" and calls Create RGB Image. And you have "1, 4, 8" which calls Create Indexed Image. In each of those VIs you have the same case structure test, splitting deeper.

First of all, we're testing the same value twice; since we've already encoded the bit values into the top level Create Image.vi, let's just go all the way and skip the middle calls to Create RGB Image.vi and Create Indexed Image.vi.

But we could also do this:

On Image.lvclass, define a dynamic dispatch VI named "Get Bit Encoding.vi". This VI would take an instance of the class as input and outputs a constant integer that is the bit depth of this class. Then on each of the leaf-level child classes (not the middle-layer classes) define override VIs that return 1, 4, 8, 24 or 32 respectively. Now build an array of each of the 5 types of leaf-level image classes and make that array a constant. Now you can write a VI that takes image data as input, loops over that array, calls Get Bit Encoding.vi and says "if the bit encoding of this class is the same as the bit encoding in the image data, then create a new instance of that class from the image data."

Now, in this version the only real advantage is that you don't have a case structure that is doing testing of the values -- you're instead asking each object "Could you encode this image data? If you can, then I'll make a new instance of you." That's much cleaner. Further, if the array, instead of being constant is a stored value (say, an LV2-style global), you could dynamically load new classes into the system and add them to the array at run time, and the code would still behave the same. Going to this length is definitely not worth your time on this project, but it is an idea that may be useful in the future.