www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Initial feedback for std.experimental.image

reply "Rikki Cattermole" <alphaglosined gmail.com> writes:
So as part of my testing of std.experimental.color I began 
writing an image ala ae.graphics style.
It's now ready for very initial feedback.
There is not many image manipulation functions or storage types. 
Let alone image loading/exporting. In fact there is no image file 
format actually working at this point.
Only the shell for a PNG one is so far written.

I believe it is ready for initial feedback because I feel it is 
moving towards "controversial" territory in its design. With the 
file format support.
I just need to know that I am going in the right direction as of 
now. There is no point in continuing the image reading/writing 
support like it is, if nobody likes it.

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

Like ae.graphics it supports ranges for manipulating of image 
storage types.
However it does also use std.experimental.allocator.

Please destroy!


Some thoughts of mine:
- std.zlib will need to support allocators
- Can std.experimental.allocator be  nogc pleaseeeee?
Jul 06 2015
next sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 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
 Use the Cartesian coordinates system using two (X and Y) size_t 
 integers
Why not just int? There is preciously little addressable images beyond 2^31. size_t has a variable size and is a source of build breaking. Also, please no unsigned integers! This create all manners of bugs because of how integer promotion work.
 A major concern for D is templated code. Template bloat as it 
 is known is when there is large amounts of templates being 
 initiated with vast array of compile time arguments. For this 
 very reason, template usage must be constrained to the bare 
 minimum. Keeping in line with other D code of highly reusable 
 code, templates will be used. Be constrained to colors (usage 
 not definition) and constructors as much as possible.
I don't think there is so much problems with Phobos levels of meta-programming. There are also way to reduce template instantiation with type-punning. To me an image library should be based on a static interface with optionally a wrapper to auto-generate the dynamic interface much like in std.allocator. Is it the case? Your library seem to be based around the dynamic 'interface' instead.
Jul 06 2015
next sibling parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 3:34 a.m., ponce wrote:
 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 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
 Use the Cartesian coordinates system using two (X and Y) size_t integers
Why not just int? There is preciously little addressable images beyond 2^31. size_t has a variable size and is a source of build breaking. Also, please no unsigned integers! This create all manners of bugs because of how integer promotion work.
I've been sold on the unsigned vs signed type issue for and only for the x and y coordinates. Although I'm a little concerned about not using ptrdiff_t instead of a specific integral type. For the time being, I'll alias it and use that instead. This will need to be decided later on. Offsets will still be size_t. As it would be too limiting.
 A major concern for D is templated code. Template bloat as it is known
 is when there is large amounts of templates being initiated with vast
 array of compile time arguments. For this very reason, template usage
 must be constrained to the bare minimum. Keeping in line with other D
 code of highly reusable code, templates will be used. Be constrained
 to colors (usage not definition) and constructors as much as possible.
I don't think there is so much problems with Phobos levels of meta-programming. There are also way to reduce template instantiation with type-punning. To me an image library should be based on a static interface with optionally a wrapper to auto-generate the dynamic interface much like in std.allocator. Is it the case? Your library seem to be based around the dynamic 'interface' instead.
I choose structs primarily for my types because it can be optimized away and also allocated on stack instead of the heap. If people prefer classes they can and are more than welcome to use them. There are interfaces for just this purpose identical to ranges. Only like ranges there is no point on just relying on it. Instead I have SwappableImage struct. Which allows with no allocation to wrap up any image storage type you wish via the constructor. I am very proud of this. http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.SwappableImage
Jul 06 2015
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole wrote:
 I've been sold on the unsigned vs signed type issue for and 
 only for the x and y coordinates.
The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
Jul 09 2015
next sibling parent reply "rikki cattermole" <alphaglosined gmail.com> writes:
On Thursday, 9 July 2015 at 23:35:02 UTC, Vladimir Panteleev 
wrote:
 On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole wrote:
 I've been sold on the unsigned vs signed type issue for and 
 only for the x and y coordinates.
The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
Canvas API != image library. I'm quite happy for the canvas API to use signed integers. While the image library does not. After all the canvas API would just wrap how its being stored. You lose the ability to have such large images as the CPU architecture can support but meh. Not my problem. The canvas API being built on top should sanitise coordinates as need be. This is not unreasonable. Little unknown fact is Devisualization.Util features a type called LineGraph which basically gets points based upon primitives such as lines and arcs. With support for deltas. https://github.com/Devisualization/util/blob/master/source/core/devisualization/util/core/linegraph.d Of course it was buggy but, I ran into the same issue you did. My view is definitely canvas should be signed. Just the underlying image storage type doesn't need to use it.
Jul 09 2015
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Friday, 10 July 2015 at 04:56:53 UTC, rikki cattermole wrote:
 On Thursday, 9 July 2015 at 23:35:02 UTC, Vladimir Panteleev 
 wrote:
 On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole 
 wrote:
 I've been sold on the unsigned vs signed type issue for and 
 only for the x and y coordinates.
The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
Canvas API != image library. I'm quite happy for the canvas API to use signed integers. While the image library does not. After all the canvas API would just wrap how its being stored.
Why must there be a distinction between canvas and image? Seems like a pointless abstraction layer.
 You lose the ability to have such large images as the CPU 
 architecture can support but meh. Not my problem.
No, that's wrong. The limit (for 32 bits) is 2 billion pixels for signed coordinates ON ONE AXIS. So the only situation in which an unsigned width/height will be advantageous is a 1-pixel-wide/tall image with a 1-byte-per-pixel-or-less color type.
 The canvas API being built on top should sanitise coordinates 
 as need be. This is not unreasonable.
I think it is. If you want to draw something 10 pixels away from the right border, the expression img.width-10 will be unsigned.
Jul 09 2015
parent reply "rikki cattermole" <alphaglosined gmail.com> writes:
On Friday, 10 July 2015 at 05:01:57 UTC, Vladimir Panteleev wrote:
 On Friday, 10 July 2015 at 04:56:53 UTC, rikki cattermole wrote:
 On Thursday, 9 July 2015 at 23:35:02 UTC, Vladimir Panteleev 
 wrote:
 On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole 
 wrote:
 I've been sold on the unsigned vs signed type issue for and 
 only for the x and y coordinates.
The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
Canvas API != image library. I'm quite happy for the canvas API to use signed integers. While the image library does not. After all the canvas API would just wrap how its being stored.
Why must there be a distinction between canvas and image? Seems like a pointless abstraction layer.
A canvas API may only be in name only. In other words, instead of it taking size_t it would take ptrdiff_t in e.g. free-functions.
 You lose the ability to have such large images as the CPU 
 architecture can support but meh. Not my problem.
No, that's wrong. The limit (for 32 bits) is 2 billion pixels for signed coordinates ON ONE AXIS. So the only situation in which an unsigned width/height will be advantageous is a 1-pixel-wide/tall image with a 1-byte-per-pixel-or-less color type.
Okay okay, I'm getting picky I know its silly.
 The canvas API being built on top should sanitise coordinates 
 as need be. This is not unreasonable.
I think it is. If you want to draw something 10 pixels away from the right border, the expression img.width-10 will be unsigned.
Okay you have a point here with img.width. Humm, complicated problem here. To the average developer having to wrap up an image type with e.g. CanvasImage before using it, even if it was a struct would just seem useless. Just to change index types to be signed. What we need is a unsigned integer type of word size that can have an out of bounds value aka negative. While also supporting calculations that is both signed and unsigned. So basically a unsigned integer behaving like a signed one. Now wouldn't that solve the problem we have?
Jul 09 2015
next sibling parent "Tofu Ninja" <emmons0 purdue.edu> writes:
On Friday, 10 July 2015 at 05:27:21 UTC, rikki cattermole wrote:
 What we need is a unsigned integer type of word size that can 
 have an out of bounds value aka negative. While also supporting 
 calculations that is both signed and unsigned.
 So basically a unsigned integer behaving like a signed one.
 Now wouldn't that solve the problem we have?
Yeah we have those, they are called signed integers... But for real, you are way over complicating the problem, just use an int.
Jul 10 2015
prev sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Friday, 10 July 2015 at 05:27:21 UTC, rikki cattermole wrote:
 What we need is a unsigned integer type **of word size**
Sorry for being picky there. At least in x86, machine word size aka 64-bit integers in 64-bit are generally not faster. - first, all operations are operating on 32-bit integers by default, unless those with a REX prefix. The important part is that _those instructions with a REX prefix take more bytes_. This has 2 effects, one on the size of the code itself (expect more icache usage), and one on the instruction decoding queue (processor can't process out of order too much long instruction, I believe the limit is a 16-byte lookahead or something). - secondly to avoid false dependencies, the upper 32-bit of registers are cleared. mov ECX, 0; // will clear the whole RCX register. So while the code size may not be a big deal in the general scheme, it has literally no downsides to use as much 32-bit operations as possible even in 64-bit modes on x86. I don't know with ARM or others.
Jul 21 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 22/07/2015 4:01 a.m., ponce wrote:
 On Friday, 10 July 2015 at 05:27:21 UTC, rikki cattermole wrote:
 What we need is a unsigned integer type **of word size**
Sorry for being picky there. At least in x86, machine word size aka 64-bit integers in 64-bit are generally not faster. - first, all operations are operating on 32-bit integers by default, unless those with a REX prefix. The important part is that _those instructions with a REX prefix take more bytes_. This has 2 effects, one on the size of the code itself (expect more icache usage), and one on the instruction decoding queue (processor can't process out of order too much long instruction, I believe the limit is a 16-byte lookahead or something). - secondly to avoid false dependencies, the upper 32-bit of registers are cleared. mov ECX, 0; // will clear the whole RCX register. So while the code size may not be a big deal in the general scheme, it has literally no downsides to use as much 32-bit operations as possible even in 64-bit modes on x86. I don't know with ARM or others.
I would rather picky begins now, not later. Having these sort of problems sorted out now, means when its time for review, you can't say I haven't argued for the design. So in your view, I should ask the compiler devs of e.g. ldc/gdc of what would be the better way to go for production code in this instance?
Jul 21 2015
parent "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 21 July 2015 at 16:27:33 UTC, Rikki Cattermole wrote:
 So in your view, I should ask the compiler devs of e.g. ldc/gdc 
 of what would be the better way to go for production code in 
 this instance?
Why not, this wasn't about std.experimental.image specifically. I just wanted to point out something that is counter-intuitive (of course this need to be cross-checked). We discussed signed vs unsigned and I think Vladimir made compelling arguments about signed integers. 32-bit vs 64-bit vs machine-size words is another to choose and I'd better complain now than at vote time :).
Jul 21 2015
prev sibling parent reply ketmar <ketmar ketmar.no-ip.org> writes:
On Thu, 09 Jul 2015 23:35:00 +0000, Vladimir Panteleev wrote:

 On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole wrote:
 I've been sold on the unsigned vs signed type issue for and only for
 the x and y coordinates.
=20 The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. =20 A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
giving that `int` and `uint` are freely interchangeable... `uint` is=20 better, as it allows only one bound check in `if`, and without casts. ;-)=
Jul 10 2015
parent reply "Vladimir Panteleev" <thecybershadow.lists gmail.com> writes:
On Friday, 10 July 2015 at 23:21:02 UTC, ketmar wrote:
 On Thu, 09 Jul 2015 23:35:00 +0000, Vladimir Panteleev wrote:

 On Tuesday, 7 July 2015 at 03:39:00 UTC, Rikki Cattermole 
 wrote:
 I've been sold on the unsigned vs signed type issue for and 
 only for the x and y coordinates.
The first version of ae.utils.graphics had unsigned coordinates. As I found out, this was a mistake. A rule of thumb I discovered is that any numbers you may want to subtract, you should use signed types. Otherwise, operations such as drawing an arc with its center point off-canvas (with negative coordinates) becomes unreasonably complicated.
giving that `int` and `uint` are freely interchangeable... `uint` is better, as it allows only one bound check in `if`, and without casts. ;-)
That's a poor reason. Optimizing x>=0 && x<100 to an unsigned check is such a basic optimization, even DMD can do it.
Jul 10 2015
parent ketmar <ketmar ketmar.no-ip.org> writes:
On Sat, 11 Jul 2015 01:51:41 +0000, Vladimir Panteleev wrote:

 giving that `int` and `uint` are freely interchangeable... `uint` is
 better, as it allows only one bound check in `if`, and without casts.
 ;-)
=20 That's a poor reason. Optimizing x>=3D0 && x<100 to an unsigned check is such a basic optimization, even DMD can do it.
but `if (x < 100)` is less typing than `if (x >=3D 0 && x < 100)`!=
Jul 10 2015
prev sibling parent reply Manu via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 7 July 2015 at 01:34, ponce via Digitalmars-d
<digitalmars-d puremagic.com> wrote:
 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 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
 Use the Cartesian coordinates system using two (X and Y) size_t integers
Why not just int? There is preciously little addressable images beyond 2^31. size_t has a variable size and is a source of build breaking. Also, please no unsigned integers! This create all manners of bugs because of how integer promotion work.
size_t doesn't quietly cast to int. Most iterators are already size_t. Indexes shoulds be size_t for conventions sake, otherwise you case everywhere. Indices should be unsigned, otherwise you need to do 2 tests for validity, rather than just one; (x >= 0 && x < length) rather than just (x < length).
Jul 07 2015
next sibling parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 8:50 p.m., Manu via Digitalmars-d wrote:
 On 7 July 2015 at 01:34, ponce via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 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
 Use the Cartesian coordinates system using two (X and Y) size_t integers
Why not just int? There is preciously little addressable images beyond 2^31. size_t has a variable size and is a source of build breaking. Also, please no unsigned integers! This create all manners of bugs because of how integer promotion work.
size_t doesn't quietly cast to int. Most iterators are already size_t. Indexes shoulds be size_t for conventions sake, otherwise you case everywhere. Indices should be unsigned, otherwise you need to do 2 tests for validity, rather than just one; (x >= 0 && x < length) rather than just (x < length).
Well you both have very valid points. In this case, lets go with: It's already written, let's not change it just because it might solve a few bugs while potentially adding others.
Jul 07 2015
prev sibling parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 7 July 2015 at 08:50:45 UTC, Manu wrote:

 Most iterators are already size_t.
.length should have been int but it's too late. It's not because it's a "size" that it should be size_t.
 Indexes shoulds be size_t for conventions sake, otherwise you 
 cast everywhere.
Disagree. I think it's the other way around. And image coordinates are not especially indices.
 Indices should be unsigned, otherwise you need to do 2 tests for
 validity, rather than just one; (x >= 0 && x < length) rather 
 than
 just (x < length).
You can force an unsigned comparison with (cast(uint)x < length). Signed indices optimize better in loops because signed overflow is undefined behaviour. http://stackoverflow.com/questions/18795453/why-prefer-signed-over-unsigned-in-c
Jul 07 2015
next sibling parent reply "David Nadlinger" <code klickverbot.at> writes:
On Tuesday, 7 July 2015 at 12:10:10 UTC, ponce wrote:
 Signed indices optimize better in loops because signed overflow 
 is undefined behaviour.

 http://stackoverflow.com/questions/18795453/why-prefer-signed-over-unsigned-in-c
Only in C/C++. In D, they are defined to overflow according to two's complement. — David
Jul 07 2015
parent "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 7 July 2015 at 14:02:53 UTC, David Nadlinger wrote:
 Only in C/C++. In D, they are defined to overflow according to 
 two's complement.

  — David
Thanks for the correction.
Jul 07 2015
prev sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
On Tue, 07 Jul 2015 12:10:07 +0000, ponce wrote:

 Signed indices optimize better in loops because signed overflow is
 undefined behaviour.
one of the reasons i dropped C. luckily, in D it's defined.=
Jul 07 2015
prev sibling next sibling parent reply "Meta" <jared771 gmail.com> writes:
On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 I believe it is ready for initial feedback because I feel it is 
 moving towards "controversial" territory in its design. With 
 the file format support.
 I just need to know that I am going in the right direction as 
 of now. There is no point in continuing the image 
 reading/writing support like it is, if nobody likes it.
For ImageStorage, why provide both pixelAt/pixelStoreAt (bad naming IMO) and opIndex/opIndexAssign?
Jul 06 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 4:55 a.m., Meta wrote:
 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 I believe it is ready for initial feedback because I feel it is moving
 towards "controversial" territory in its design. With the file format
 support.
 I just need to know that I am going in the right direction as of now.
 There is no point in continuing the image reading/writing support like
 it is, if nobody likes it.
For ImageStorage, why provide both pixelAt/pixelStoreAt (bad naming IMO) and opIndex/opIndexAssign?
So it can be used here: http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.SwappableImage Reasons why this could have been a problem: http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.ImageStorage.opIndex vs http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.ImageStorageOffset.opIndex In other words, overloads. Operator overloads here are just syntax candy which is required for everyday use. But the functions are required so they can be used as delegate pointers in other types. If you want another name, please suggest them! After all, I only came up with it at like 3am.
Jul 06 2015
next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Tuesday, 7 July 2015 at 03:41:59 UTC, Rikki Cattermole wrote:
 On 7/07/2015 4:55 a.m., Meta wrote:
 For ImageStorage, why provide both pixelAt/pixelStoreAt (bad 
 naming IMO)
If you want another name, please suggest them! After all, I only came up with it at like 3am.
getPixel/putPixel or a variation of such? This is the most common name for such functions.
Jul 09 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 10/07/2015 11:41 a.m., Vladimir Panteleev wrote:
 On Tuesday, 7 July 2015 at 03:41:59 UTC, Rikki Cattermole wrote:
 On 7/07/2015 4:55 a.m., Meta wrote:
 For ImageStorage, why provide both pixelAt/pixelStoreAt (bad naming IMO)
If you want another name, please suggest them! After all, I only came up with it at like 3am.
getPixel/putPixel or a variation of such? This is the most common name for such functions.
Fine by me.
Jul 09 2015
parent reply "Tofu Ninja" <emmons0 purdue.edu> writes:
On Friday, 10 July 2015 at 03:00:57 UTC, Rikki Cattermole wrote:
 getPixel/putPixel or a variation of such? This is the most 
 common name
 for such functions.
Fine by me.
This is honestly just nitpicking, but I see setPixel more than putPixel I think.
Jul 09 2015
next sibling parent "rikki cattermole" <alphaglosined gmail.com> writes:
On Friday, 10 July 2015 at 03:59:32 UTC, Tofu Ninja wrote:
 On Friday, 10 July 2015 at 03:00:57 UTC, Rikki Cattermole wrote:
 getPixel/putPixel or a variation of such? This is the most 
 common name
 for such functions.
Fine by me.
This is honestly just nitpicking, but I see setPixel more than putPixel I think.
I'm fine with it. I'd rather just get it over and done with now and not let this happen later in review period.
Jul 09 2015
prev sibling parent reply ketmar <ketmar ketmar.no-ip.org> writes:
On Fri, 10 Jul 2015 03:59:29 +0000, Tofu Ninja wrote:

 On Friday, 10 July 2015 at 03:00:57 UTC, Rikki Cattermole wrote:
 getPixel/putPixel or a variation of such? This is the most common name
 for such functions.
Fine by me.
=20 This is honestly just nitpicking, but I see setPixel more than putPixel I think.
i'm using `setPixel` to change pixel unconditionally (think of it as "set=20 rgb"), and `putPixel` to blend pixel (think of it as "do rgba color mix").=
Jul 10 2015
parent reply "Meta" <jared771 gmail.com> writes:
On Friday, 10 July 2015 at 23:23:32 UTC, ketmar wrote:
 On Fri, 10 Jul 2015 03:59:29 +0000, Tofu Ninja wrote:

 On Friday, 10 July 2015 at 03:00:57 UTC, Rikki Cattermole 
 wrote:
 getPixel/putPixel or a variation of such? This is the most 
 common name for such functions.
Fine by me.
This is honestly just nitpicking, but I see setPixel more than putPixel I think.
i'm using `setPixel` to change pixel unconditionally (think of it as "set rgb"), and `putPixel` to blend pixel (think of it as "do rgba color mix").
Wouldn't that better be called blendPixel?
Jul 10 2015
next sibling parent "Tofu Ninja" <emmons0 purdue.edu> writes:
On Saturday, 11 July 2015 at 00:49:27 UTC, Meta wrote:
 Wouldn't that better be called blendPixel?
Also wouldn't a blendPixel need to know the blending scheme, alpha, premultiplied, additive, ect...
Jul 10 2015
prev sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
On Sat, 11 Jul 2015 00:49:25 +0000, Meta wrote:

 i'm using `setPixel` to change pixel unconditionally (think of it as
 "set rgb"), and `putPixel` to blend pixel (think of it as "do rgba
 color mix").
=20 Wouldn't that better be called blendPixel?
as Tofu said, `blendPixel` should then be parameterized with blending=20 mode. and for `putPixel` i know blending mode, as it is fixed. anyway, that was only a suggestion and (poor) reasoning for it, i'm not=20 insisting on that scheme.=
Jul 10 2015
prev sibling parent reply "Suliman" <evermind live.ru> writes:
Hi! Is there any progress?
Sep 03 2015
next sibling parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 03/09/15 10:28 PM, Suliman wrote:
 Hi! Is there any progress?
Indeed! Quite a bit really. https://github.com/rikkimax/alphaPhobos/tree/master/source/std/experimental/graphic/image PNG should now be import/exportable. I have not ehh tested this just yet however. Building a test framework for it will be a rather major task and could take ~9 hours at least. I believe I'll be working on skewing today on stream, in about an hour. https://www.livecoding.tv/alphaglosined/ My focus isn't too much on the image library. More so on all the other code in that repository. My time line still has about a year on it till I'm feeling ready to start the PR process after all :)
Sep 03 2015
prev sibling parent "Rikki Cattermole" <alphaglosined gmail.com> writes:
On Thursday, 3 September 2015 at 10:28:50 UTC, Suliman wrote:
 Hi! Is there any progress?
Update, during todays stream. Instead of adding scew manipulation instead adding range clever support functions (createImageFrom and assignTo). ImageImpl image1; // input ImageImpl[2] image2; // buffers write("myfile.png", image1.rangeOf .map!(p => { p.y += p.x * addY; p.height = addY * p.height; }) .createImageFrom!ImageImpl(image2[0], theAllocator()) .assignTo(image2[0], RGB8(255, 255, 255)) .rangeOf .flipHorizontalRange .rotate90Range .flipVerticalRange .createImageFrom!ImageImpl(image2[1], theAllocator()) .copyInto(image2[1]) .asPNG.toBytes() ); theAllocator().dispose(image[0]); theAllocator().dispose(image[1]);
Sep 03 2015
prev sibling next sibling parent reply "Tofu Ninja" <emmons0 purdue.edu> writes:
On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 So as part of my testing of std.experimental.color I began 
 writing an image ala ae.graphics style.
 It's now ready for very initial feedback.
 There is not many image manipulation functions or storage 
 types. Let alone image loading/exporting. In fact there is no 
 image file format actually working at this point.
 Only the shell for a PNG one is so far written.

 I believe it is ready for initial feedback because I feel it is 
 moving towards "controversial" territory in its design. With 
 the file format support.
 I just need to know that I am going in the right direction as 
 of now. There is no point in continuing the image 
 reading/writing support like it is, if nobody likes it.

 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

 Like ae.graphics it supports ranges for manipulating of image 
 storage types.
 However it does also use std.experimental.allocator.

 Please destroy!


 Some thoughts of mine:
 - std.zlib will need to support allocators
 - Can std.experimental.allocator be  nogc pleaseeeee?
For my uses, the only thing that I really care about is file saving and loading. Personally I would prefer if the lib depended on something that already works well for image loading and storing like freeImage. It has way more file types that have been proven to work than I would reasonably expect would be implemented if they were done in pure D. If people reject the idea of the std lib depending on an outside lib, then I would at least ask that the design be such that an outside lib could be easily used in conjunction with std.image.
Jul 06 2015
next sibling parent reply "Suliman" <evermind live.ru> writes:
If people reject
 the idea of the std lib depending on an outside lib, then I 
 would at least ask that the design be such that an outside lib 
 could be easily used in conjunction with std.image.
Yes it's not good idea, because you need one freeImage, I need gdal image support, but all of us need any standard graphical toolkit at last for simple tasks. So if std.image will help to get it a lot of people would very thanks full.
Jul 06 2015
parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 6:04 a.m., Suliman wrote:
 If people reject
 the idea of the std lib depending on an outside lib, then I would at
 least ask that the design be such that an outside lib could be easily
 used in conjunction with std.image.
Yes it's not good idea, because you need one freeImage, I need gdal image support, but all of us need any standard graphical toolkit at last for simple tasks. So if std.image will help to get it a lot of people would very thanks full.
My end goal is at least by the end of next year is to have Devisualization.Window moved into Phobos. From there it is only a matter of time till somebody comes up with a GUI toolkit. After all they have the ability to create windows + context on all three main platforms and can draw anything they like.
Jul 06 2015
prev sibling parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 5:47 a.m., Tofu Ninja wrote:
 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 So as part of my testing of std.experimental.color I began writing an
 image ala ae.graphics style.
 It's now ready for very initial feedback.
 There is not many image manipulation functions or storage types. Let
 alone image loading/exporting. In fact there is no image file format
 actually working at this point.
 Only the shell for a PNG one is so far written.

 I believe it is ready for initial feedback because I feel it is moving
 towards "controversial" territory in its design. With the file format
 support.
 I just need to know that I am going in the right direction as of now.
 There is no point in continuing the image reading/writing support like
 it is, if nobody likes it.

 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


 Like ae.graphics it supports ranges for manipulating of image storage
 types.
 However it does also use std.experimental.allocator.

 Please destroy!


 Some thoughts of mine:
 - std.zlib will need to support allocators
 - Can std.experimental.allocator be  nogc pleaseeeee?
For my uses, the only thing that I really care about is file saving and loading. Personally I would prefer if the lib depended on something that already works well for image loading and storing like freeImage. It has way more file types that have been proven to work than I would reasonably expect would be implemented if they were done in pure D. If people reject the idea of the std lib depending on an outside lib, then I would at least ask that the design be such that an outside lib could be easily used in conjunction with std.image.
At the current design, as long as you import the implementation and then call the function related to it, you are good to go. I.E. import std.experimental.freeimage; auto myImage = ...; write("filename", myImage.asFreeImage().toBytes("png")); // ubyte[] Or atleast that is the idea of it. Although it definitely would not be done in Phobos. The whole point is to give a common interface for the D community and to enable swapping these images around. Manipulating them as needed.
Jul 06 2015
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Tuesday, 7 July 2015 at 03:46:55 UTC, Rikki Cattermole wrote:
 write("filename", myImage.asFreeImage().toBytes("png")); //
Um, does this line mean that code to support ALL image formats in the library is going to end up in the executable? Selecting which image formats are supported by the application should be an explicit choice done by the application code.
Jul 09 2015
parent "rikki cattermole" <alphaglosined gmail.com> writes:
On Thursday, 9 July 2015 at 23:44:06 UTC, Vladimir Panteleev 
wrote:
 On Tuesday, 7 July 2015 at 03:46:55 UTC, Rikki Cattermole wrote:
 write("filename", myImage.asFreeImage().toBytes("png")); //
Um, does this line mean that code to support ALL image formats in the library is going to end up in the executable? Selecting which image formats are supported by the application should be an explicit choice done by the application code.
I'm not using FreeImage for loading/exporting of images. It was an example for another library to manage it. Instead of purely D one. You will be free to use any you wish as long as you can import it. Currently there is no big bad manager for registering of image types because of M:N problem that is registering templated functions while not knowing the template arguments at compile time.
Jul 09 2015
prev sibling next sibling parent reply "Baz" <bb.temp gmx.com> writes:
On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 So as part of my testing of std.experimental.color I began 
 writing an image ala ae.graphics style.
 It's now ready for very initial feedback.
I don't see some things that immediatly come to my mind as "usefull" when i think to the image class as it's been done elsewhere. - Resampling API ? e.g the old trick to get AA: draw on double sized image canvas and resample to halh using an interpolator. image.loadFromFile(...); image.resize(horzRatio, vertRatio, Resampler.linear); image.saveToFile(...); - Canvas API ? myImage.canvas.pen.color = RGBA!ubyte(...); myImage.canvas.fillRect(...); etc. needed to programmatically generate an image.
Jul 06 2015
parent reply ketmar <ketmar ketmar.no-ip.org> writes:
On Mon, 06 Jul 2015 20:16:30 +0000, Baz wrote:

 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 So as part of my testing of std.experimental.color I began writing an
 image ala ae.graphics style.
 It's now ready for very initial feedback.
=20 I don't see some things that immediatly come to my mind as "usefull" when i think to the image class as it's been done elsewhere. =20 - Resampling API ? =20 e.g the old trick to get AA: draw on double sized image canvas and resample to halh using an interpolator. =20 image.loadFromFile(...); image.resize(horzRatio, vertRatio, Resampler.linear); image.saveToFile(...); =20 - Canvas API ? =20 myImage.canvas.pen.color =3D RGBA!ubyte(...); myImage.canvas.fillRect(...); etc. =20 needed to programmatically generate an image.
i believe this is out of scope of the library. that is exactly the things=20 that should be build latter with image library as foundation.=
Jul 06 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 2:28 p.m., ketmar wrote:
 On Mon, 06 Jul 2015 20:16:30 +0000, Baz wrote:

 On Monday, 6 July 2015 at 13:48:53 UTC, Rikki Cattermole wrote:
 So as part of my testing of std.experimental.color I began writing an
 image ala ae.graphics style.
 It's now ready for very initial feedback.
I don't see some things that immediatly come to my mind as "usefull" when i think to the image class as it's been done elsewhere. - Resampling API ? e.g the old trick to get AA: draw on double sized image canvas and resample to halh using an interpolator. image.loadFromFile(...); image.resize(horzRatio, vertRatio, Resampler.linear); image.saveToFile(...); - Canvas API ? myImage.canvas.pen.color = RGBA!ubyte(...); myImage.canvas.fillRect(...); etc. needed to programmatically generate an image.
i believe this is out of scope of the library. that is exactly the things that should be build latter with image library as foundation.
Resizing the likes suggested are manipulation functions. Not out of scope, I most likely will not be implementing them. My goal is only to do the simpler image manipulation functions as I would have to implement them twice each. One to work with ranges the other to whole sale modify the image to be that way and may allocate in the process. Canvas is definitely and 100% out of scope however. It's a great idea and something we need long term. Just not something I can do right now. Also need to add that to specification document as a follow on work and out of scope.
Jul 06 2015
parent ketmar <ketmar ketmar.no-ip.org> writes:
On Tue, 07 Jul 2015 15:51:13 +1200, Rikki Cattermole wrote:

 Canvas is definitely and 100% out of scope however. It's a great idea
 and something we need long term. Just not something I can do right now.
 Also need to add that to specification document as a follow on work and
 out of scope.
so what primitives you want to include? will there be line joints of=20 various types? wide lines (more that 1px wide)? will it allow=20 antialiasing, and if yes, will it be at least of the quality of anti- grain geometry? etc, etc. do you already see why i believe that canvas is out of scope? it's simply=20 too big task that should be done separately, using image lib as=20 foundation.=
Jul 07 2015
prev sibling next sibling parent reply ketmar <ketmar ketmar.no-ip.org> writes:
On Mon, 06 Jul 2015 13:48:51 +0000, Rikki Cattermole wrote:

 Please destroy!
have no real opinion, but for me everything is looking good. i don't=20 really care about "ideal design", only about "usable design". and that=20 seems usable.
 Some thoughts of mine:
 - std.zlib will need to support allocators
std.zlib need to stop being crap. i.e. it should be thrown away and=20 completely rewritten. i did that several times, including pure D inflate=20 and compress/decompress that using byte streams. never used std.zlib as=20 is.=
Jul 06 2015
next sibling parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 7/07/2015 2:33 p.m., ketmar wrote:
 On Mon, 06 Jul 2015 13:48:51 +0000, Rikki Cattermole wrote:

 Please destroy!
have no real opinion, but for me everything is looking good. i don't really care about "ideal design", only about "usable design". and that seems usable.
 Some thoughts of mine:
 - std.zlib will need to support allocators
std.zlib need to stop being crap. i.e. it should be thrown away and completely rewritten. i did that several times, including pure D inflate and compress/decompress that using byte streams. never used std.zlib as is.
In that case, ketmar is now in charge of writing a new std.compression module for Phobos! Since he already knows these algos well. We can sort out who does the PR itself into Phobos. Most likely I'll have to. We can do reviews on the N.G. instead of on Github just so that he can be responsible for it ;) And yes, I'll happily add it as a dependency.
Jul 06 2015
parent ketmar <ketmar ketmar.no-ip.org> writes:
On Tue, 07 Jul 2015 15:54:15 +1200, Rikki Cattermole wrote:

 In that case, ketmar is now in charge of writing a new std.compression
 module for Phobos! Since he already knows these algos well.
i don't think that i'm ready to code for phobos. coding style can be=20 fixed with dfmt (yet it will require to backport patches from other=20 people, grrrr), but no tool can fix lack of documentation and tests. and=20 i really HAET writing those. the same for writing API parts that should=20 be there "for completeness", but has no use for me. so while i can build a draft, some other dedicated person must do all the=20 other work. and if there is such dedicated person, that person can do=20 that without my draft shitcode, for sure. p.s. writing xpath and css selectors engine for Adam's "dom.d" is much=20 funnier, as i can stop caring about "api completeness", "comprehensive=20 documentation" and so on. if my engine will be able to do more that=20 current "dom.d" engine do, Adam will be happy to merge it, i believe. ;-)=
Jul 07 2015
prev sibling parent reply Manu via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 7 July 2015 at 12:33, ketmar via Digitalmars-d
<digitalmars-d puremagic.com> wrote:
 On Mon, 06 Jul 2015 13:48:51 +0000, Rikki Cattermole wrote:

 Please destroy!
have no real opinion, but for me everything is looking good. i don't really care about "ideal design", only about "usable design". and that seems usable.
 Some thoughts of mine:
 - std.zlib will need to support allocators
std.zlib need to stop being crap. i.e. it should be thrown away and completely rewritten. i did that several times, including pure D inflate and compress/decompress that using byte streams. never used std.zlib as is.
What's wrong with zlib? Surely it's possible to produce a d interface for zlib that's nice? Thing is, zlib is possibly the single most popular library in the world, and it gets way more testing, and improvements + security fixes from time to time. Why wouldn't you want to link to the real thing?
Jul 07 2015
next sibling parent "rsw0x" <anonymous anonymous.com> writes:
On Tuesday, 7 July 2015 at 08:54:57 UTC, Manu wrote:
 On 7 July 2015 at 12:33, ketmar via Digitalmars-d 
 <digitalmars-d puremagic.com> wrote:
 [...]
What's wrong with zlib? Surely it's possible to produce a d interface for zlib that's nice? Thing is, zlib is possibly the single most popular library in the world, and it gets way more testing, and improvements + security fixes from time to time. Why wouldn't you want to link to the real thing?
D community suffers from NIH syndrome
Jul 07 2015
prev sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
On Tue, 07 Jul 2015 18:54:45 +1000, Manu via Digitalmars-d wrote:

 std.zlib need to stop being crap. i.e. it should be thrown away and
 completely rewritten. i did that several times, including pure D
 inflate and compress/decompress that using byte streams. never used
 std.zlib as is.
=20 What's wrong with zlib? Surely it's possible to produce a d interface for zlib that's nice? Thing is, zlib is possibly the single most popular library in the world, and it gets way more testing, and improvements + security fixes from time to time. Why wouldn't you want to link to the real thing?
nothing wrong with *zlib*, it's *std.zlib* sux. but not `etc.c.zlib`=20 interface, which i used in some of my stream codecs.=
Jul 07 2015
prev sibling next sibling parent reply "Gregor =?UTF-8?B?TcO8Y2tsIg==?= <gregormueckl gmx.de> writes:
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. - 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. - 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). - 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. 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. - 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. - 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. Let me know what you think!
Jul 08 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
On 9/07/2015 6:07 a.m., "Gregor =?UTF-8?B?TcO8Y2tsIg==?= 
<gregormueckl 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.
Jul 08 2015
parent reply "Gregor =?UTF-8?B?TcO8Y2tsIg==?= <gregormueckl gmx.de> writes:
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:
 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*.
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.
 - 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.
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.
 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.
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.
 - 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.
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?
 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.
Jul 09 2015
parent reply Rikki Cattermole <alphaglosined gmail.com> writes:
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:
 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*.
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.
 - 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.
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.
 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.
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.
 - 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.
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.
Jul 09 2015
next sibling parent reply =?UTF-8?B?Ik3DoXJjaW8=?= Martins" <marcioapm gmail.com> writes:
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 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.
Jul 09 2015
parent "rikki cattermole" <alphaglosined gmail.com> writes:
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.
Jul 09 2015
prev sibling parent reply "Gregor =?UTF-8?B?TcO8Y2tsIg==?= <gregormueckl gmx.de> writes:
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:
 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*.
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.
I haven't encountered SwappableImage anywhere. Did I miss anything?
 - 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.
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.
Again, I unfortunately fail to follow. Sorry. I wanted to argue that window regions are out of scope, unlike subimages (which actually are completely separate images within the same file - they do not share a single coordinate system).
 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.
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.
Messy in what way? My conceptual view of the user code is that it could do either of two things: 1. ask for a image in a format the user specifies - the loader must meet that request exactly and do all required conversions 2. ask the loader for the image data format contained in the file, choose a representation that best matches it in the user's view and only then ask the loader as in 1. Note that the user gets exactly what he asked for in each case, but makes an informed decision in 2. only because he chose to.
 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.
That's what this would boil down to, I believe: a separate entry point for the loader that just parses the headers and returns file format information to the user, but not the image data itself. The user-facing interface would just be an extra interface method. Internally, it would share most of the code with the actual image loading code path.
 - 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.
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.
Am I looking at an old version? The PNG loader I see documented returns a specific ImageStorage implementation.
 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...
OK, why do you abstract image storage at all? What's the rationale there? Storage formats other than the trivial row-by-row storage are only used in very few cases in my experience, mostly when it involves compression (compressed textures for games - effectively read only after compression) or certain kinds of out-of-core image processing. This seems to be mostly out of scope this library IMO.
 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.
This is a really big can of worms, especially if you want to set standards for the future. You can do anything from informal and lax to extreme level of detail. For example, we require students to write specifications that include numbered lists of *all* functional and non-functional requirements for their (rather small) projects, which fill endless pages in the end. Additionally they have to describe the development tools and environment, the target audience, manual and automatic tests, and so on. It's the classic waterfall model with all the problems it has (don't ask - decision from the top...). In your case I'd start by defining a target audience and describing target use cases in a high-level manner (e.g. "identify the format and required loader for an image file", "load the image so that the user sees a representation which he understands", ...). Stating non-goals or non-features is a good thing to limit scope (your spec already mentions some). The list of use cases can be checked against relevance for the target audience. Also, the use cases can be translated into test cases of various kinds: - example snippets demonstrating ease of use of the design (or lack thereof) - automatic tests (e.g. unit tests) - manual tests of complex scenarios requiring user interaction (not relevant in this case, but e.g. for UI libraries) A very formal approach could number the use cases and tests and keep cross-references between them to make it easier to check for completeness. But for the most part, this would be tedious and boring for little gain.
Jul 12 2015
parent Rikki Cattermole <alphaglosined gmail.com> writes:
On 13/07/2015 1:43 a.m., "Gregor =?UTF-8?B?TcO8Y2tsIg==?= 
<gregormueckl gmx.de>" 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:
 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*.
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.
I haven't encountered SwappableImage anywhere. Did I miss anything?
It's only used for image formats right now directly. http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.SwappableImage
 - 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.
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.
Again, I unfortunately fail to follow. Sorry. I wanted to argue that window regions are out of scope, unlike subimages (which actually are completely separate images within the same file - they do not share a single coordinate system).
What I mean by subimage is basically a region of another image. Like a sprite in a sprite sheet. Same idea.
 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.
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.
Messy in what way? My conceptual view of the user code is that it could do either of two things: 1. ask for a image in a format the user specifies - the loader must meet that request exactly and do all required conversions 2. ask the loader for the image data format contained in the file, choose a representation that best matches it in the user's view and only then ask the loader as in 1. Note that the user gets exactly what he asked for in each case, but makes an informed decision in 2. only because he chose to.
Think of it like this: void myfunc(string name) { import std.experimental.image.fileformats.png; ForwardRange!ubyte input = ...; auto headers = loadPNGHeaders(input.save()); if (headers.pixelType == PNGPixelType.RGB && headers.samples == 16) { write("output.png", loadPNG!RGB16(input.save()) .flipHorizontalRange .flipVerticalRange . ... copyTemporary!MyImage(allocator) .asPNG .toBytes); } else if (headers.pixelType == PNGPixelType.RGBA && headers.samples == 16) { write("output.png", loadPNG!RGBA16(input.save()) .flipHorizontalRange .flipVerticalRange . ... copyTemporary!MyImage(allocator) .asPNG .toBytes); } } Of course that was off the top of my head in more pseudo code then anything else. It's also late and I'm tired so excuse any wrong terminology.
 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.
That's what this would boil down to, I believe: a separate entry point for the loader that just parses the headers and returns file format information to the user, but not the image data itself. The user-facing interface would just be an extra interface method. Internally, it would share most of the code with the actual image loading code path.
Okay, I'm long since convinced its needed. So no worries here.
 - 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.
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.
Am I looking at an old version? The PNG loader I see documented returns a specific ImageStorage implementation.
It doesn't? SwappableImage is used in its place. Also it is wrapped in a PNGFileFormat because file format != image storage type. So while yes it behaves as an image. It is not an image primarily. As it includes the headers.
 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...
OK, why do you abstract image storage at all? What's the rationale there? Storage formats other than the trivial row-by-row storage are only used in very few cases in my experience, mostly when it involves compression (compressed textures for games - effectively read only after compression) or certain kinds of out-of-core image processing. This seems to be mostly out of scope this library IMO.
Your right. There is only three that really matter to most users. One of the primary users of the library is for game development. It is who I care personally care about. There is also a lot of overlap between efficient GUI toolkit's and game development for images. While implementations beyond the big three, horizontal/vertical scan lines and one dimensional array are out of scope, the extension isn't.
 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.
This is a really big can of worms, especially if you want to set standards for the future. You can do anything from informal and lax to extreme level of detail. For example, we require students to write specifications that include numbered lists of *all* functional and non-functional requirements for their (rather small) projects, which fill endless pages in the end. Additionally they have to describe the development tools and environment, the target audience, manual and automatic tests, and so on. It's the classic waterfall model with all the problems it has (don't ask - decision from the top...). In your case I'd start by defining a target audience and describing target use cases in a high-level manner (e.g. "identify the format and required loader for an image file", "load the image so that the user sees a representation which he understands", ...). Stating non-goals or non-features is a good thing to limit scope (your spec already mentions some). The list of use cases can be checked against relevance for the target audience. Also, the use cases can be translated into test cases of various kinds: - example snippets demonstrating ease of use of the design (or lack thereof) - automatic tests (e.g. unit tests) - manual tests of complex scenarios requiring user interaction (not relevant in this case, but e.g. for UI libraries) A very formal approach could number the use cases and tests and keep cross-references between them to make it easier to check for completeness. But for the most part, this would be tedious and boring for little gain.
Thank you. I'm definitely staring this to read later tomorrow when working on it on stream. It'll also help with dismissing the arguments regarding signed/unsigned vs set size/word size of the coordinate type.
Jul 12 2015
prev sibling next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
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?
Jul 09 2015
parent reply "rikki cattermole" <alphaglosined gmail.com> writes:
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.
Jul 09 2015
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Friday, 10 July 2015 at 04:35:51 UTC, rikki cattermole wrote:
 write("output.png", image
 .flipHorizontalRange
 .flipVerticalRange
 .rotate90Range
 .copyInto(new TheImage!RGB8(2, 2))

 .asPNG().toBytes);
Why the "Range" suffix for functions? Aren't most functions going to be lazy, and thus have a "Range" suffix? Does copyInto do resizing / colorspace conversion? Such operations should be explicit. If it doesn't, then most of that sub-expression is redundant... Have you looked at ae.utils.graphics.image.copy? If a second argument is not provided, an appropriate image is allocated automatically (on the heap though).
 I have an idea similar to copyInto except:
 .copyIntoTemporary!TheImage(allocator) // return 
 SwappableImage!(ImageColor!TheImage)(allocator.make!TheImage, 
 allocator); // auto ref return type
It took me a while to understand what SwappableImage does. The documentation could be improved. I think you should not be adding runtime polymorphism implicitly anywhere. There are virtual range interfaces in Phobos, yes, but wrapping is only done explicitly, as it should be here.
 Basically SwappableImage can already deallocate the original 
 storage instance when it's destructor gets called. If provided 
 with the allocator. Same with RangeOf.
I suggest to avoid conflating Phobos range terminology with 2D image views. The current naming scheme makes me think that it returns a range of rows, or some other range which is directly pluggable into std.algorith. What's wrong with "View"? I'd also like to ask why you decided to write a library from scratch instead of reusing existing ones. I've spent days solving certain design issues of ae.utils.graphics, it seems wasteful to me to discard that and duplicate the effort. If the license is the issue, I'd be happy to relicence it.
Jul 09 2015
parent "rikki cattermole" <alphaglosined gmail.com> writes:
On Friday, 10 July 2015 at 04:52:54 UTC, Vladimir Panteleev wrote:
 On Friday, 10 July 2015 at 04:35:51 UTC, rikki cattermole wrote:
 write("output.png", image
 .flipHorizontalRange
 .flipVerticalRange
 .rotate90Range
 .copyInto(new TheImage!RGB8(2, 2))

 .asPNG().toBytes);
Why the "Range" suffix for functions? Aren't most functions going to be lazy, and thus have a "Range" suffix?
There are three kinds of mutation functions per functionality. 1) Modifies an image in place and returns it 2) image.rangeOf.function 3) Takes in a PixelPoint input range Range suffix seemed like a good way to separate out which return a range and which don't.
 Does copyInto do resizing / colorspace conversion? Such 
 operations should be explicit. If it doesn't, then most of that 
 sub-expression is redundant...
Nope. It just copies a PixelPoint input range into an image storage type.
 Have you looked at ae.utils.graphics.image.copy? If a second 
 argument is not provided, an appropriate image is allocated 
 automatically (on the heap though).

 I have an idea similar to copyInto except:
 .copyIntoTemporary!TheImage(allocator) // return 
 SwappableImage!(ImageColor!TheImage)(allocator.make!TheImage, 
 allocator); // auto ref return type
It took me a while to understand what SwappableImage does. The documentation could be improved. I think you should not be adding runtime polymorphism implicitly anywhere. There are virtual range interfaces in Phobos, yes, but wrapping is only done explicitly, as it should be here.
In this case the only reason SwappableImage would be used is to use its lifetime management facilities. But hey you can manage it yourself if you want too...
 Basically SwappableImage can already deallocate the original 
 storage instance when it's destructor gets called. If provided 
 with the allocator. Same with RangeOf.
I suggest to avoid conflating Phobos range terminology with 2D image views. The current naming scheme makes me think that it returns a range of rows, or some other range which is directly pluggable into std.algorith. What's wrong with "View"?
As far as I am aware the range capabilities could be used with std.algorithm. There is nothing stopping you. It just uses PixelPoint!Color to represent a single pixel instead an instance of the color. http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_interfaces.html#.PixelPoint
 I'd also like to ask why you decided to write a library from 
 scratch instead of reusing existing ones. I've spent days 
 solving certain design issues of ae.utils.graphics, it seems 
 wasteful to me to discard that and duplicate the effort. If the 
 license is the issue, I'd be happy to relicence it.
Licensing isn't an issue. I choose to write a new one to take advantage of std.experimental.color. It originally was meant to just test it. In fact I've been taking a lot of queues from ae.utils.graphics for this very reason. It is a very good piece of code and it already has solved may of the issues already. Although now it is also pretty much testing e.g. std.experimental.allocators too. I chose against reusing ae.utils.graphics as a base (same as e.g. dlib's and Devisualization.Image) because it would take a lot of work to get working with the newer Phobos to be modules. Almost as much as a new library would take. You were the only person I was really interested in opinion wise. Manu's is great, but you have already done this. You know what works and doesn't.
Jul 09 2015
prev sibling parent "Rikki Cattermole" <alphaglosined gmail.com> writes:
Bumpity bump.
Updated.

Includes most of the PNG implementation for loading of images.
http://cattermole.co.nz/phobosImage/docs/html/std_experimental_image_fileformats_png.html
I've also added a little bit more to the specification document.

Next stream I'll be tackling IDAT chunk loading (not something I 
exactly enjoy).
Jul 20 2015