OP_INVERT and anti-aliasing don't play together

[Pete]

This is a long-standing glitch that I think ought to be looked into. I worked around it in my own software, but I see it shows up in e.g. ArtPaint, too.

In BeOS, the common and convenient way to draw 'rubber-band' lines, temporary outlines, and so on, was to draw with OP_INVERT: draw once and it it visible, draw again and it is erased. With everything being drawn anti-aliased, this no longer works. Things don't get erased properly, and garbage is left strewn everywhere.

When I found this happening in MusicWeaver, I wrote my own short Bresenham code to explicitly draw with single points (actually "InvertRect(BRect(x,y,x,y));") but it would be nicer to be able to use the kit's methods to draw lines, and especially other things. ArtPaint, for example, draws its temporary rectangles and ellipses with OP_INVERT.

Would it be reasonable to simply inhibit anti-aliasing if OP_INVERT is selected? It's probably the only situation in which that mode is used. Haven't looked at the code, but I suspect it wouldn't be hard to implement.

[Pete]

Any chance of prioritizing this a bit -- before beta?

ArtPaint, for example, would still be a useful program if rubber-banding worked cleanly. (It's very good for a lot of bitmap work. It has other bugs in Haiku, but they're not really worth looking into at the moment.)

And I've just discovered that SyncModular -- which otherwise seems to be fine in Haiku -- also suffers from smearing lines. As it's proprietary, it's not fixable.

I don't feel competent at digging into that code myself, but I can't imagine it's too hard to suppress anti-aliasing for OP_INVERT. Be really nice to have it fixed.

[pulkomandy]

I don't even know if agg can disable antialiasing, and how.

Anyway, if we want to keep antialiasing enabled for new apps, we have some support to do this in an app-by-app basis or by detecting BeOS apps. I don't know if this is useful or if we always want to disable it.

[Pete]

I'm not suggesting that anti-aliasing should be disabled for anything but OP_INVERT. I don't see that OP_INVERT is useful for anything except rubber-banding and similar reversible drawing ops, and it doesn't work at all for that with anti-aliasing active. If you know of any app that uses the mode for anything else (and needs anti-aliasing for it), please share! (:-))

I have to admit that I got lost trying to trace the code, so I don't actually know how difficult it would be to disable it based on mode, but isn't anti-aliasing an extension of simpler point-by-point stroking?

[pulkomandy]

Yes, of course we won't disable antialiasing for normal drawing. Some of the other modes may need similar changes, however.

For reference, the B_OP_INVERT implementation is at ​http://cgit.haiku-os.org/haiku/tree/src/servers/app/drawing/Painter/drawing_modes/DrawingModeInvert.h and ​http://cgit.haiku-os.org/haiku/tree/src/servers/app/drawing/Painter/drawing_modes/DrawingModeInvertSUBPIX.h (for B_SUBPIXEL_PRECISE views).

It looks like removing the calls to BLEND_INVERT could work. Alternatively, we could tweak that function so that its operation is reversible, so we can get antialiasing without leaving artifacts on the background.

[Pete]

Replying to pulkomandy:

Yes, of course we won't disable antialiasing for normal drawing. Some of the other modes may need similar changes, however.

OK, thanks. Wasn't quite sure what you were suggesting. I think the problem is exclusive to OP_INVERT. Thanks for the pointers to code

It looks like removing the calls to BLEND_INVERT could work. Alternatively, we could tweak that function so that its operation is reversible, so we can get antialiasing without leaving artifacts on the background.

Making the anti-aliasing version reversible would be perfect. Not sure it's possible, though? The original op was unique in that applying it twice restores the original state. Can that happen with a "scaled" inversion? I suppose if it's an exclusive-or it would be OK.

[pulkomandy]

The inversion is not done with an XOR, but with a negation: d[0] = 255 - _p.data8[0];. This is also a reversible operation: 255 - (255 - x) = 255 - 255 + x = x.

This formula is used when the drawing fully covers a pixel. When there is only partial coverage, that new color is blended with the previous one: BLEND(d, 255 - _p.data8[2], 255 - _p.data8[1], 255 - _p.data8[0], a); (with a being the coverage percentage).

The BLEND macro does: d[0] = ((((b - _p.data8[0]) * a) + (_p.data8[0] << 8)) >> 8); (with "b" the new color, "a" the coverage, and _p the initial value of the pixel). Blending in this way is not a self-reversible operation. I don't know if we can design a reversible blend for this purpose.

[Pete]

Replying to pulkomandy:

It looks like removing the calls to BLEND_INVERT could work. Alternatively, we could tweak that function so that its operation is reversible, so we can get antialiasing without leaving artifacts on the background.

Looking more closely, I don't think just removing BLEND_INVERT would do the job. If you examine the pixels of an anti-aliased, slightly-slanted, line, you can see fairly large sections where there is no 100% (a=255) coverage of any written pixel. You'd get gaps in the drawing. As OP_INVERT is being fed the anti-aliased data, there's not much can be done about that.

I think letting BLEND_INVERT draw pixels above a certain threshold coverage would work. It seems reasonable that 50% (>127) is probably the right threshold.

(Thinking more about some kind of "scaled invertible blend", I think you're right and that it's not possible. Trying to invert less than all the bits leads to crazy values.)

Also, I don't think that setting alpha to 255, as the current code does, is correct. This is not reversible. I'd prefer that alpha was just left alone.

So here's a suggested minimal rewrite of thos macros. (I've switched to exclusive-or because I can't see any problem.)

// BLEND_INVERT
// (threshold may be arbitrary.  leave alpha unchanged)
#define BLEND_INVERT(d, a) \
 if (a > 127) { \
	d[0] = 255 ^ d[0]; \
	d[1] = 255 ^ d[1]; \
	d[2] = 255 ^ d[2]; \
}

// ASSIGN_INVERT
// (leave alpha unchanged)
#define ASSIGN_INVERT(d) \
{ \
	d[0] = 255 ^ d[0]; \
	d[1] = 255 ^ d[1]; \
	d[2] = 255 ^ d[2]; \
}

Obviously the threshold check could be done in the invoking code rather than the macro, and the two could probably be merged a bit, but I don't have the ability to build a test ATM, so I'll leave it there.

[stippi]

It's probably a good idea to change this as Pete suggests. It would be more compatible also in other apps, like in GobeProductive's text selection and blinking cursor.

I did definitely use B_OP_INVERT in some of my apps with anti-aliasing. It looks nice, but of course you cannot use it as a reversible operation. So these apps would simply not look as good as they do now, but it shouldn't result in any actual problems like with the old apps now.

[Pete]

Replying to stippi:

I did definitely use B_OP_INVERT in some of my apps with anti-aliasing. It looks nice, but of course you cannot use it as a reversible operation. So these apps would simply not look as good as they do now, but it shouldn't result in any actual problems like with the old apps now.

I'm certainly surprised that you found a use for B_OP_INVERT other than for easy reversing. Can you give a brief explanation?

If an anti-aliased version is useful for some things, can't we just add another drawing_mode (B_OP_INVERT_BLEND or something)?