[boost] Re: A preliminary proposal: AGG project

From: Maxim Shemanarev (mcseem_at_[hidden])
Date: 2002-05-01 22:20:52

----- Original Message -----
From: "Jens Maurer" <Jens.Maurer_at_[hidden]>
Newsgroups: gmane.comp.lib.boost.devel
Sent: Wednesday, May 01, 2002 1:02 PM
Subject: Re: A preliminary proposal: AGG project

Thanks a lot! It's really helpful. I'll try to explain some points in order
to
receive more criticism :-)

> I won't look at the library any further until at least
> some documentation is available.

Quite right. Still I'd ask you to read these notes at least :-)

>  - The distribution .zip should contain the documentation so
> that people can read it offline.
>
>  - You ought to fix the links on your docs web page.
> The documentation was mostly inaccessible.  The doxygen
> documentation does not appear to have any explanation
> in it.

Precisely! I'm working on it.

>  - For a 2D vector graphics library, portability is a
> major concern, since underlying graphics primitives vary
> a lot by platform.

The primary rendering techique in AGG is to use a buffer in memory.
It does'n mean you cannot adapt it for a number of underlying APIs, but
there're some concerns. First of all it's an ability to accept subpixel
coordinates and to draw primitives in accordance with it, i.e., two calls
line(0.0, 0.0, 10.0, 5.0) and line(0.0, 0.45, 10.0, 4.55), must activate
different sets of pixels. Here pixel coordinates are assumed.
Second is anti-aliased rendering. As far as I know there're not so many
APIs that support these, most of them round off the coordinates to integers
in low level before rendering and it actually gives great inaccuracy.
Subpixel coordinates and anti-aliasing are two tightly connected things.

If you want to adapt the library to use with some existing graphical
API that works only in integer pixel coordinates, you still can use the
conversion pipelines part of the library. The idea of it is explained at
antigrain:
http://www.antigrain.com/agg_docs/doc_pipelines.html
(sorry, but there's too much text to put it here).

But in this case we also have problems. AGG has its own rendering
mechanism similar to glyph renderer in its idea, and it assumes
that the only low level primitive we have is polygon. We have
to calculate 4 outline points even if we need to draw a single line segment.
This approach allows you to render perfect images (and it's pretty
fast), but it doesn't make much sense if we have some API with a lot
of existing primitives, such as line, circle, ellipse and so on.

I do not try to implement a universal wrapper library for any type of
existing API, because this is practically impossible and will be
ugly, like Microsoft ODBS, for example.

What I try to do is to create a rendering library similar to
existing ones, such as LibArt (http://www.levien.com/libart/) but
much better, faster, with new ideas, and completely C++ed.

> I'd certainly like to see an adaptation
> to X11, first because I don't use Win32, and second because
> it allows to check whether portability concerns have been
> addressed successfully in the AGG design.

AGG does not rely on any particular API, such as Win32
(except the examples so far).
If you want to use it with X11 you will need to display pixel
images generated with AGG. And it's your responsibility
to allocate a buffer for the image. You have an impression
that AGG is win32 dependent. It's my fault and it's because
the only I have for now is Win32 platform.

>  - It appears your library uses RGB (and Alpha) as the
> color scheme.  What about other color schemes such as YUV?

Color space (color scheme) has two meanings. One is
a possibility to set colors in different color spaces and quite
another is to render images directly in a particular color space,
i.e. to have a canvas represented in, say, YUV (Y'Cb'Cr). First
is very simple and it will be solved by a number of functions of
converting different color spaces to a single one, say, RGB.

Another thing is to work directly in YUV, HSV or others.
Yes, the primary canvas color spaces are RGB, CMYK
and Grayscale (for now only RGB implemented).
The criterium of using a particular color space as a canvas is:
if it's easy to calculate mixed colors it can be used in AGG.
For example, the colorspace must be continuous if you want to
have anti-aliasing, named color sets (palettes) won't do.

> If there's really only one color scheme in the 2D vector
> graphics application domain, I'd be surprised, i.e.
> it should be documented, with rationale.

It will be. Sorry, this is in process now.

> Design
> ======
>  - I don't understand why your constructors are
> often relatively empty, and then you provide an init()
> or create() member function (see, for example, agg::ellipse).
> I could envision performance reasons to provide an init()
> function *in addition* to a full-fledged constructor (that
> avoids tearing down the object).
>  - The STL often seems to avoid using set_ and get_ prefixes
> for member functions, so poly.thickness(5) seems to be
> more in line with this than poly.set_thickness(5).  In particular
> since there does not appear to be a poly.get_thickness().
> (see iostreams, for example).

Agree. I'll do that.

>  - A canvas appears to be named render_bgr24_solid.  I've so
> far only seen "rgb", never the reverse, even though bits
> might probably be stored in "bgr" and not "rgb" order.

It's named always after the physical representation of a pixel
in this particular format. So, there will be a number of classes
to represent different pixel formats and different color spaces.
I have to solve a very difficult task: How to produce a good design
with keeping the maximum performance. For example, I cannot
afford to use any virtual calls on the low level. The idea is to
create a number of classes that depend on a pixel format and
color space (there can be many of them, but they're simple)
that work as efficient as possible. Often you use only
one color space and pixel format, so the template mechanism
solves this perfectly. If you need some general case that
could work with many pixel formats simultaneously, you can
do that using a common base class with virtual finctions
considering that it will cost you some overhead.
Again, the idea is to design the "algorithmic" classes without
any common base class and without any virtuals, and then, if
you really need, to use thin wrappers that provide classical
polymorfism mechanism.

>  - An interface using "reset_iterator" and "next_vertex"
> is exactly not STL-like, because it does not allow two
> iterators on the same underlying container-like object.
> The STL uses separate iterator classes and a begin() / end()
> interface on the container, and operator++() on the iterator.
> This allows dealing with subranges.

This point I'd like to discuss it as detailed as possible.
The problems are:
1. All conv_ classes are not containers. They're more
likely iterators. The idea of the pipelines is not to
store intermediate results (with some exceptions), but to perform
as much calculations on demand as possible.
2. In general case it's impossible to have direct access to vertices
and it's hard to use subranges - see above. I cannot use operator [].
For example, if you can easily use subranges with linked lists in STL,
you cannot do that with curve_iterator (it's possible, but see clause 5).
3. I agree it would be better to be more STL-friendly, but.
Paths are the main idea of rendering with conversion pipelines and
path storage can contain many paths with many thousands of vertices.
I use a custom mechanism of allocations in order not to reallocate
memory, reallocs are evil (again, efficiency vs genericity)
Every path has an ID that is passed to reset_iterator, it's a very generic
and very poweful mechanism, so, it cannot be replaces to begin()
without arguments.
4. The idea is to have a number of classes-converters that support a
very simple interface of only two functions and that can be combined
into any pipeline. Separation these classes into converters and their
iterators will make constructing the pipelines much harder.
5. Converter classes are very special case of iterators and I
haven't ever needed to access the vertices directly or to store
the value of the iterator and then return to it. The question is do we
really need this extra complexity only on order to look similar to STL?

> Filenames
> =========

>  - At boost, we tend to use subdirectories instead of prefixes
> for header filenames.  We haven't had trouble with that so
> far, as long as all includes (even internal sub-includes)
> use the full relative path, e.g. (in your case) agg/something.h
> I believe that using subdirectories helps organization.
>
>  - There is no need to have filename prefixes in .cpp
> implementation files.

Almost agree. Still, there are some problems. If you have files
with the the name, some silly Unix debuggers (dbx) cannot distinguish
one file from another even if they are in different subdirectories.
On the other hand, there's no requirement in Boost gudelines to
avoid prefixes in filenames.

>  - It is unfortunate that all examples appear to be
> Win32-only, and MSVC wizard-generated also.  There does not
> seem to be a "Makefile" or other means around that would
> allow users of Borland, Intel or Metrowerks compilers to
> compile the examples.  I suggest using only bare-bones
> features of the Win32 native library (no fancy AFX lib)
> to open a window, and then pass control to the 2D library
> to show its features (probably a controlled "quit" from
> the example app needs a little event handling from the
> underlying platform also).

Agree completely. But I really need help with it. I'm planning to
create some barebone for win-32 but for now it's all I can.
AFX sucks and my fault is that I haven't been concentrating on
these things yet.

> It would show the portability of your library if several
> target platforms such as Win32 and X11 could share the 2D
> implementation, with only minimal surrounding platform-dependent
> boilerplate.  Currently, the inherently Win32-specific OnDraw()
> member function seems to have all the 2D code within it.
> See  http://www.boost.org/more/imp_vars.htm.

Yes, I have read this. Again, all will be done, but it requires time.

> - Your example code occasionally uses home-grown forward
> declarations (IdeaView.h), which seems questionable design
> to me.  If such forward declarations are indeed useful, there
> should be a library header provided that has these.
>    namespace agg
>    {
>        class render_bgr24_solid;
>    }

Thanks a lot! I will polish it as well as using "Koenig" in examples.

>  - The class pixel_map isn't even in the doxygen documentation.

Because it's not supposed to be there. This is the only windows-dependent
class, it's just a helper and not a part of AGG core. I'm planning to have
similar classes to other platforms and describe it separately.

> Implementation
> ==============
>  -     void render_bgr24_solid::render(scanline* sl)
> uses a pointer, although the implementation clearly assumes
> that "sl" cannot be 0.  Wouldn't a reference be more
> appropriate here?

Maybe. I just try to follow the princilpe "all reference arguments
must be const, if they're not use pointers".
In this case scanline cannot be const.

>  - At that same function, calculations using >> 16 etc.
> appear to assume a certain bit width for built-in
> datatypes such as "char".  Are you certain that you can
> deal with 32bit "char"s here?

You're right, it won't work. This class depends on a particular
pixel format, namely, 8-bit B-G-R with a possibility to have
access to each component independently. Hardware platforms
that have 32-bit chars (I know only Crey so far) are considered
to have different pixel format even if it's also B-G-R. Pixel
format means an implementation of access to pixels in the
most efficient way. If I try to implement these critical things
absolutely independently on the platform it can seriously affect
the performance. In my opinion it's better to implement another
simple class to deal with these exotic platforms.

This is the most useful reply about AGG I've ever seen.
Keep criticizing me!
Special thanks for compilation!

McSeem
http://www.antigrain.com