Initial feedback for std.experimental.image (page 4)

On Friday, 10 July 2015 at 04:26:49 UTC, Vladimir Panteleev wrote:
> On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
>> Docs: http://cattermole.co.nz/phobosImage/docs/html/
>> Source: http://cattermole.co.nz/phobosImage/
>>
>> If reviewing the code itself is too much of a hassel, please review the specification document. http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_specification.html
>
> Some example code using this library would be much more concise than the above. Can you provide some?

I haven't really gotten to that point.
But currently what I want it to support is:

write("output.png", image
.flipHorizontalRange
.flipVerticalRange
.rotate90Range
.copyInto(new TheImage!RGB8(2, 2))

.asPNG().toBytes);

I have an idea similar to copyInto except:
.copyIntoTemporary!TheImage(allocator) // return SwappableImage!(ImageColor!TheImage)(allocator.make!TheImage, allocator); // auto ref return type

Basically SwappableImage can already deallocate the original storage instance when it's destructor gets called. If provided with the allocator. Same with RangeOf.

July 10, 2015

Re: Initial feedback for std.experimental.image

Posted by rikki cattermole
in reply to Márcio Martins

Permalink

rikki cattermole

Posted in reply to Márcio Martins

Permalink

I drafted a reply to this, then had time to think about it. So new answer here.

On Thursday, 9 July 2015 at 16:10:28 UTC, Márcio Martins wrote:
> On Thursday, 9 July 2015 at 15:05:12 UTC, Rikki Cattermole wrote:
>> On 10/07/2015 2:07 a.m., "Gregor =?UTF-8?B?TcO8Y2tsIg==?= <gregormueckl@gmx.de>" wrote:
>>> On Thursday, 9 July 2015 at 04:09:11 UTC, Rikki Cattermole wrote:
>>>> On 9/07/2015 6:07 a.m., "Gregor =?UTF-8?B?TcO8Y2tsIg==?=
>>>> <gregormueckl@gmx.de>" wrote:
>>>>> [...]
>>>>
>>>> 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*.
>>>>
>>>
>>> Hm... in that case you introduce transparent mappings between
>>> user-facing types and the internal mapping which may be lossy in various
>>> ways. This works, but the internal type should be discoverable somehow.
>>> This leads down a similar road to OpenGL texture formats: they have
>>> internal storage formats and there's the host formats to/from which the
>>> data is converted when passing back and forth. This adds a lot of
>>> complexity and potential for surprises, unfortunately. I'm not entirely
>>> sure what to think here.
>>
>> Internal color to an image storage type is well known at compile time.
>> Now SwappableImage that wraps another image type. That definitely muddies the water a lot. Since it auto converts from the original format. Which could be, you know messy.
>> It's actually the main reason I asked Manu for a gain/loss precision functions. For detecting if precision is being changed. Mostly for logging purposes.
>>
>>>>> [...]
>>>>
>>>> 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.
>>>>
>>>
>>> Yes, this is a slightly special use case. I can think of quite a lot of
>>> cases where you would want border regions of some kind for what you are
>>> doing, but they are all related to rendering and image processing.
>>
>> You have convinced me that I need to add a subimage struct which is basically SwappableImage. Just with offset/size different to original.
>>
>>>>> [...]
>>>>
>>>> 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.
>>>>
>>>
>>> I can understand your reasoning and this is why libraries like FreeImage
>>> make it very simple to get the image data converted to the format you
>>> want from an arbitrary input. What I'd like to see is more of an
>>> extension of the current mechanism: make it possible to query the data
>>> format of the image file. That way, the application can make a wiser
>>> decision on the format in which it wants to receive the data, but it
>>> always is able to get the data in a format it understands. The format
>>> description for the file format would have to be quite complex to cover
>>> all possibilities, though. The best that I can come up with is a list of
>>> tuples of channel names (as strings) and data type (as enums).
>>> Processing those isn't fun, though.
>>
>> The problem here is simple. You must know what color type you are going to be working with. There is no guessing. If you want to change to match the file loader better, you'll have to load it twice and then you have to understand the file format internals a bit more.
>> This is kinda where it gets messy.
>>
>> But, would it be better if you could just parse the headers? So it doesn't initialize the image data. I doubt it would be all that hard. It's just disabling a series of features.
>>
>>>>> [...]
>>>>
>>>> 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.
>>>>
>>>>> [...]
>>>>
>>>> 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.
>>>>
>>>
>>> This is where the abstraction of ImageStorage with several possible
>>> implementations becomes iffy. The user is at the loader's mercy to
>>> hopefully hand over the right implementation type. I'm not sure I like
>>> that idea. This seems inconsistent with making the pixel format the
>>> user's choice.  Why should the user have choice over one thing and not
>>> the other?
>>
>> If the image loader uses another image storage type then it is miss behaving. There is no excuse for it.
>>
>> Anyway the main thing about this to understand is that if the image loader does not initialize, then it would have to resize and since not all image storage types have to support resizing...
>>
>>>>
>>>> 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.
>>>>
>>>
>>> I mentioned OpenImageIO as this library is full-featured and very
>>> complete in a lot of areas. It shows what it takes to be as flexible as
>>> possible regarding the image data that is processed. Take it as a
>>> catalog of things to consider, but not as template.
>>>
>>>> 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.
>>>
>>> Having a specification is a good thing and this is why I entered the
>>> discussion. Although your specification is still a bit vague in my
>>> opinion, the general direction is good. The limitation of the scope
>>> looks fine to me and I'm not arguing against that. My point is rather
>>> that your design can still be improved to match that scope better.
>>
>> Yeah indeed. Any tips for specification document improvement?
>> I would love to make it standard for Phobos additions like this.
>
> Like Gregor, I think it's unreasonable to do any automatic conversions at all without being ask to do. This will greatly reduce the usability of this library.

We have very different views about being asked to convert.
SwappableImage does it, if the color type is not the same. File exporters only do it if the fields in the header ask it to (or will). But file readers are the only use case right now that doesn't.

What I've been sold on is separating out reading of headers from the reading of the data optionally. In other words, the general use case it'll just read and give data as normal. Along with auto convert capabilities. But for the use case you are saying it will read only headers (not templated as it does not need to know what color type you are using). Then you can read the file again (with data) using the appropriate color type.

It'll be interesting how I solve this since they are intertwined a little too much. But hey void is a good color type internally right? Although a struct would probably be a better idea as it gives a nice name to the purpose. Or even an interface *shrugs*.

> We need to solve the problem of getting from a file format on disk into a color format in memory. I can get from an image that I have already stored and preprocessed in a format I like, and I want to get it as quickly as possibly into a GPU buffer. Similarly, there are many use cases for an image library that do not touch individual pixels at all, so doing any sort of conversion at load time is basically telling those people to look elsewhere, if they care about efficiency.
>
>
> The most efficient way is a low-level 2-step interface:
> 1. Open the file and read headers (open from disk, from a memory buffer, or byte range)
> - At this point, users know color format width, and image dimensions, so they can allocate their buffers, check what formats the GPU supports or just otherwise assess if conversion is needed.
> 2. Decode into a user supplied buffer, potentially with color format conversion, if requested. This is important.
>
> At this point, we have a buffer with known dimensions and color format.
> Some very useful manipulations can be achieved without knowing anything about the color format, except for the bit-size. Examples are flipping, rotations by a multiple of PI/2, cropping, etc...
>
> On top of this, one can create all sorts of easy to use functions for all the remaining use cases, but this sort of low level access is really important for any globally useful library. Some users just cannot afford any sort of extra unnecessary copying and or conversions.
>
> I also think we should be able to load all the meta information on demand. This is extremely valuable, but the use-cases are so diverse that it doesn't make sense to implement more than just discovering all this meta-data and letting users do with it what they will.
>
> The most import thing is to get the interface right and lightweight.
> If we can get away with no dependencies then it's even better.

Forums