Initial feedback for std.experimental.image

Rikki Cattermole via Digitalmars-d digitalmars-d at puremagic.com
Wed Jul 8 21:09:09 PDT 2015


On 9/07/2015 6:07 a.m., "Gregor =?UTF-8?B?TcO8Y2tsIg==?= 
<gregormueckl at gmx.de>" wrote:
> On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
>>
>> Please destroy!
>>
>
> You asked for it! :)
>
> As a reference to a library that is used to handle images on a
> professional level (VFX industry), I'd encourage you to look at the
> feature set and interfaces of OpenImageIO. Sure, it's a big library and
> some of it is definitely out of scope for what you try to accomplish
> (image tile caching and texture sampling, obviously).
>
> Yet, there are some features I specifically want to mention here to
> challenge the scope of your design:
>
> - arbitrary channel layouts in images: this is a big one. You mention 3D
> engines as a targeted use case in the specification. 3D rendering is one
> of the worst offenders when it comes to crazy channel layouts in
> textures (which are obviously stored as image files). If you have a data
> texture that requires 2 channels (e.g. uv offsets for texture lookups in
> shaders or some crazy data tables), its memory layout should also only
> ever have two channels. Don't expand it to RGB transparently or anything
> else braindead. Don't change the data type of the pixel values wildly
> without being asked to do so. The developer most likely has chosen a 16
> bit signed integer per channel (or whatever else) for a good reason.
> Some high end file formats like OpenEXR even allow users to store
> completely arbitrary channels as well, often with a different
> per-channel data format (leading to layouts like RGBAZ with an
> additional mask channel on top). But support for that really bloats
> image library interfaces. I'd stick with a sane variant of the
> uncompressed texture formats that the OpenGL specification lists as the
> target set of supported in-memory image formats. That mostly matches
> current GPU hardware support and probably will for some time to come.

As long as the color implementation matches isColor from 
std.experimental.color. Then it's a color. I do not handle that :)
The rest of how it maps in memory is defined by the image storage types. 
Any image loader/exporter can use any as long as you specify it via a 
template argument *currently*.

> - padding and memory alignment: depending on the platform, image format
> and task at hand you may want the in-memory layout of your image to be
> padded in various ways. For example, you would want your scanlines and
> pixel values aligned to certain offsets to make use of SIMD instructions
> which often carry alignment restrictions with them. This is one of the
> reasons why RGB images are sometimes expanded to have a dummy channel
> between the triplets. Also, aligning the start of each scanline may be
> important, which introduces a "pitch" between them that is greater than
> just the storage size of each scanline by itself. Again, this may help
> speeding up image processing.

Image storage type implementation, not my problem. They can be added 
later to support padding ext.

> - subimages: this one may seem obscure, but it happens in a number
> common of file formats (gif, mng, DDS, probably TIFF and others).
> Subimages can be - for instance - individual animation frames or
> precomputed mipmaps. This means that they may have metadata attached to
> them (e.g. framerate or delay to next frame) or they may come in totally
> different dimensions (mipmap levels).

Ahhh, this. I can add it fairly easily. A struct that is a sub image of 
another. Any extra data would have to be alias this'd.

> - window regions: now this not quite your average image format feature,
> but relevant for some use cases. The gist of it is that the image file
> may define a coordinate system for a whole image frame but only contain
> actual data within certain regions that do not cover the whole frame.
> These regions may even extend beyond the defined image frame (used e.g.
> for VFX image postprocessing to have properly defined pixel values to
> filter into the visible part of the final frame). Again, the OpenEXR
> documentation explains this feature nicely. Again, I think this likely
> is out of scope for this library.

Ugh based upon what you said, that is out of scope of the image 
loader/exporters that I'm writing. Also right now it is only using 
unsigned integers for coordinates. I'm guessing if it is outside the 
bounds it can go negative then.
Slightly too specialized for what we need in the general case.

> My first point also leads me to this criticism:
>
> - I do not see a way to discover the actual data format of a PNG file
> through your loader. Is it 8 bit palette-based, 8 bit per pixel or 16
> bits per pixel? Especially the latter should not be transparently
> converted to 8 bits per pixel if encountered because it is a lossy
> transformation. As I see it right now you have to know the pixel format
> up front to instantiate the loader. I consider that bad design. You can
> only have true knowledge of the file contents after the image header
> were parsed. The same is generally true of most actually useful image
> formats out there.

The reasoning is because this is what I know I can work with. You 
specify what you want to use, it'll auto convert after that. It makes 
user code a bit simpler.

However if you can convince Manu Evans to add some sort of union color 
type that can hold many different sizes for e.g. RGB. Then I'm all for 
it. Although I would consider this to be a _bad_ feature.

> - Could support for image data alignment be added by defining a new
> ImageStorage subclass? The actual in-memory data is not exposed to
> direct access, is it? Access to the raw image data would be preferable
> for those cases where you know exactly what you are doing. Going through
> per-pixel access functions for large image regions is going to be
> dreadfully slow in comparison to what can be achieved with proper
> processing/filtering code.

I ugh... had this feature once. I removed it as if you already know the 
implementation why not just directly access it?
But, if there is genuine need to get access to it as e.g. void* then I 
can do it again.

> - Also, uploading textures to the GPU requires passing raw memory blocks
> and a format description of sorts to the 3D API. Being required to
> slowly copy the image data in question into a temporary buffer for this
> process is not an adequate solution.

Again for previous answer, was possible. No matter what the image 
storage type was. But it was hairy and could and would cause bugs in the 
future. Your probably better off knowing the type and getting access 
directly to it that way.


Some very good points that I believe definitely needs to be touched upon 
where had.

I've had a read of OpenImageIO documentation and all I can say is irkkk.
Most of what is in there with e.g. tiles and reflection styles methods 
are out of scope out right as they are a bit specialized for my liking. 
If somebody wants to add it, take a look at the offset support. It was 
written as an 'extension' like ForwardRange is for ranges.

The purpose of this library is to work more for GUI's and games then 
anything else. It is meant more for the average programmer then 
specialized in imagery ones. It's kinda why I wrote the specification 
document. Just so in the future if somebody comes in saying its awful 
who does know and use these kinds of libraries. Will have to understand 
that it was out of scope and was done on purpose.


More information about the Digitalmars-d mailing list