Иван Комиссаров schreef op 19-5-2015 om 19:43:
Let me describe current API and the ideas behind it.
The core class is the ImageDocument - this is the replacement of the
QImageReader/QImageWriter. Reader and writer has similar API and
implemented in the same way, so i’ve decided to merge them into one
class. ImageDocument inherits QObject so it can connect to the
QIODevice signals and read image while it downloads from network, for
example. So, device() and fileName() methods are similar to the
corresponding methods in reader/writer.
ImageDocument doesn't need to inherit QObject in order to listen to
QIODevice signals of course. First of all, it could have rather than be
a QObject, and second of all you can connect to almost anything callable
nowadays. I'm not saying that being a QObject is a bad thing per se,
just that your argument for it isn't all that convincing to me.
bool hasError()/QString error() - no comments.
Are you sure a QString as an error() is the best choice? There are other
places in the Qt API where an enum is used instead. I think an enum
would probably be a better choice: it is easier to deal with in
errorhanding (no string comparisons needed; localization issues).
bool open(OpenMode) is an attempt to get rid of the
QImageReader::canRead() function. Unlike canRead(), open can not only
check if data is valid, but also read the header of an image so we can
know how many data acually present. OpenMode can be Read/Write or
both; right now, WriteMode can only be checked by handler if it
supports writing (i.e. can be used instead of QImageIOPlugin::CanWrite).
Would it then not be better to be explicit about this, and just call it
like that: bool readHeader()? I don't think it is a good idea to try to
emulate the QIODevice API for something that isn't one. It is just
confusing IMHO. What happens if the ImageDocument can, but the
underlying QIODevice cannot write?
close() is symmetrical for open() not sure if it really needed.
So what does it actually do?
capabilities() method returns capabilities (yep) of a format, see
CapabilityFlag enum.
Hmm? The capabilities of the document? Don't the capabilities belong to
the format instead? Judging from the API you have presented so far, I
got the impression that an ImageDocument could represent image
containers in any format. Now, it seems like the capabilities of the
format are leaking into the document. How do you construct a document
from scratch then? Where do the capabilities come from? Can I use
ImageDocument to convert between two different formats?
Conceptually, ImageDocument is a 2D array (of size is
mipmapCount()*frameCount()) of ImageResources plus meta data
(ImageMeta class). The first dimension of the array is mipmaps
(SupportMipmaps flag - can be used by Ico, Icns, DDS image formats),
the second dimension is the frame number (actually, that should be
something like array index) - can be used by GIF, DDS (yes, dds have
both mipmaps and frames).
ImageResource can be a single image, a cube texture
(Capabilities::SupportSides flag) or a VoumeTexture
(Capabilities::SupportSlices flag). Type is described by
ImageResource::Type enum. In the first case, Resource contains a
single image, in the second case - array of 6 images (for each side of
a cube) and in the third case it contains array of images of size
ImageResouce::depth.
I am wondering if this is really the right slicing of the possible
contents. Pages were already mentioned before. Do you really need to be
able to have the document represent a 2D array of vectors of images (in
case you are using frames, mipmaps with volumne textures)? Are there
image formats like that that we need to support in Qt? It also results
in an overly complicated setup for the simple use cases I think.
Might it not be better to have ImageDocument contain a collection of
QImages directly, that can have different kinds of relationships with
each other and that you can somehow query for if you need to?
For an application I am working on at work, I actually wrote a document
class a bit like that. The document contains a series of images: images
made of the left or right eye, from different angles, in different wave
lengths (IR, green laser, color - and these are actually really
different types of images!) in any combination. The set can contain any
of these, but need not contain all. We keep them all in a single
ImageCollection that you can simply query for things like "which eyes
occur in the set" but also filter back using a method filtered() with
overloads for all the different dimentions we have, returning just
another image collection. Perhaps ImageDocument could work in a similar
way to collapse pages, frames, layers and mipmaps into?
Also, both ImageResource and ImageDocument have metadata associated
with it (ImageMeta class). The duplication is needed for 2 reasons:
first, some formats can support different meta for each resource
(icns), some has the same meta for all resources (DDS); second, meta
in document can be used if you don’t need to set meta for each
resource separately.
ImageDocument::supportsOption() returns options supported by the
document, supportsResourceOption() returns options supported by
resources (typically, resource options depend on the subtype of a
resource, at least for Icns).
What i dislike now is that the Document a 2D array. Maybe i should
move mipmaps dimension to a separate class Mimpap {QVector frames;} ?
Next, i don’t have support for layered images (PSD?). With current
API, they can be implemented in 2 ways - either ImageResource is a
Layer containing list of elements with layer metadata (position on a
layer, blending options, z-order). Or ImageResource is an element on a
layer and the layer it belongs is an index/name in resource’s metadata.
Well... if you want to support layers, then perhaps you need to support
much more. For instance: are you sure that all layers are bitmap-type
layers? How are layers blended? Perhaps all that goes a bit too far for
starters.
André
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
