Bounty proposal: Paint

Don’t forget how much longer some words are in other languages, particularly German.

I wonder how they’d cope with the “Set OR AND EOR” radio buttons at the top. They already barely fit. Could that be why ROL got rid of them? :)

What would it be, something like “Einstellen Oder Und X. Oder”? Maybe that’s why Acorn seemed to leave more space around those buttons than the others!

This is, of course, assuming that the words used are the same. And is y in Spanish, but that may only be for “and” in a sentence. I think Or, And, Eor is Disyunción, Conjunción, and Disyunción exclusiva… which would require rather more space. :-)

Would it be more acceptable if the tool window just stopped taking the focus when a user clicks a tool button? That way, if you clicked on, for example, the text tool, it wouldn’t put the caret in the text box unless you click in it.

Well, Artworks does this (you have to click in the toolbar’s writable icons first to get the caret for them). But ArtWorks has its own UI code precisely to handle this sort of thing, and I’m not sure how easy it would be to implement for Paint. Probably best to leave things as they are, wrt writeable icons – if they’re thought to be needed, then best to make them behave like regular writeable icons.

the OK button [is] necessary because of the way the brushes are made in the code

Yeah, I thought it probably was. I still prefer ‘Set’, though, if there’s room.

[X] Shape [ ] Stamp [ ] Tint

Works for me!

Brush Tool Enhancements Available to Test

It’s not complete, but if anyone wants to have a look what I’ve been up to with the Brush Tool Enhancements, you can now fire up this test build.

It’s an ArcFS archive. The !Paint program is inside Paint.debug, built for IOMD (Risc PC / RPCEmu) but you can build from source if you want to. My code’s still a little messy as I haven’t ironed out all the issues yet.

It has the following new features:

• Enhanced Brush tool ideas:
  • Add opacity slider
  • Make it more user friendly (the typing in sprite/shape names interface isn’t ideal, especially for the builtin brushes)
  • Tint brush sprite by currently selected colour when “shape” option unticked, i.e. use a ColourTrans Transfer Function

• The brushes are now displayed on a scrollable window pane of buttons.
• Clicking a brush button immediately selects that brush with the current tool window settings.
• You can also drag one or more sprites from a Sprite File window onto the brush pane to add them and the first will be selected.
• If you drag a sprite file onto the brush pane, the file will be opened in Paint and the first sprite is added as a brush.
• You can also still create a brush by typing in the sprite name and clicking OK, as in old versions of Paint. If the brush is already on the brush pane, it will scroll to make the button visible.

Known Issues:

1. The transparent “colour” doesn’t work at all when the opacity slider’s turned down at the moment.
2. When doing translucent brush painting, the sprite file window thumbnail doesn’t currently update immediately.
3. There’s a very strange bug, when Opacity < 255, where the first click of the brush results in the brush sprite being plotted 8 pixels (usually) offset to the right. Subsequent plots always work normally (e.g. drag the mouse 1 pixel over and it will plot correctly, regardless of zoom level). I’m working on a test program to try and isolate the cause of this bug.
4. Changes in opacity take effect immediately. I want to move the OK button next to Name and have the Scales and Shape / Stamp / Tint take effect immediately too (contrary to what I wrote earlier, I think that will work OK).
5. It’s possible there are circumstances where the temporary layer drawn over the sprite during a translucent brush stroke isn’t merged onto the underlying sprite (perhaps palette changes, mode changes, keyboard shortcuts to save or print the sprite without releasing the mouse pointer over the window work area). Further testing is needed.
6. At the moment, if you delete a sprite that’s one of the brushes on the brushes pane, stuff will break.
7. Typing a new opacity value doesn’t actually change the opacity nor move the slider yet.
8. As previously described, translucent brushes don’t work on 256-colour sprites unless they have palettes. I still need to code a prompt about this and to disable Opacity for <= 16 colours.
9. You might find it a bit slow. It currently frees the translucent layer every single time the mouse button is lifted and mallocs it again on the next click, which reduces the memory requirements but could cause heap fragmentation. Now the brush uses a ColourTrans transfer function, I found I had to drop and recreate translation tables every time the function’s workspace changes (which is needed every time the selected colour or Shape / Stamp / Tint option changes) so this may cause a slowdown on 256-colour sprites. I don’t know why this is necessary; perhaps ColourTrans makes its own copy of the transfer function workspace.

Have fun!

Would it be more acceptable if the tool window just stopped taking the focus when a user clicks a tool button? That way, if you clicked on, for example, the text tool, it wouldn’t put the caret in the text box unless you click in it.

Well, Artworks does this (you have to click in the toolbar’s writable icons first to get the caret for them). But ArtWorks has its own UI code precisely to handle this sort of thing, and I’m not sure how easy it would be to implement for Paint. Probably best to leave things as they are, wrt writeable icons – if they’re thought to be needed, then best to make them behave like regular writeable icons.

Chris it occurred to me that Paint could give the caret back to whatever window previously had the focus whenever you switch to a different tool. Also clicking OK (or Set if we rename it) on the brush tool or clicking one of my new brush buttons could also give back the caret. I’ll have to have a re-read of the Style Guide to see if that’s acceptable behaviour.

Thanks Andy! Great work. The brush pane is particularly nice. After a brief play:

• I’m assuming the Opacity slider might end up being useful for other tools too. As such, I think it would be better placed just under the Plot Action (Set/OR/AND/EOR) control, as its function is somewhat similar. It then also looks like a proper (potential) global option, rather than being buried in the Brush-specific settings.
• It would be great to have up and down buttons for the Opacity control, though I can see that space is at a premium! Not sure how possible this is. Could an ‘opacity’ icon be used instead of the text? Something like one of these? Happy to work on one for you, if you like this idea.
• The Tab and Return keypresses in the writable icons are incorrect. Paint has never done this properly, but I think it would be good to fix here. The caret should start in the topmost writable in the pane, and move down through the icons using the Tab key. Return should activate the default button, wherever the caret currently is.
• There seems to be a bit of flicker when the brush sprites are selected in the pane. Is there an unnecessary redraw going on?
  I’ll keep playing with it. Out of interest, has your new code solved the long standing occasional crash when selecting a brush? I haven’t triggered it yet, but it would fantastic to get that solved as part of this work.

Another bounty for layers? ;)

it occurred to me that Paint could give the caret back to whatever window previously had the focus whenever you switch to a different tool. Also clicking OK (or Set if we rename it) on the brush tool or clicking one of my new brush buttons could also give back the caret.

That’s interesting. My initial feeling is that just switching tools shouldn’t move the input focus around – that feels somewhat random. But maybe the use of an OK button to set the options for a particular tool might be fine – you’re indicating that you’ve set all the parameters and now you’re ready to paint with it. But I’m not really sure, to be honest. Possibly something to bear in mind once the UI for the new tools is settled.

Glad you like it :)

Opacity slider might end up being useful for other tools too. As such, I think it would be better placed just under the Plot Action (Set/OR/AND/EOR) control, as its function is somewhat similar.

Funny you should say that, as I was considering doing that too. I probably will now. I’ll be adding it to the spray can tool too soon, as per the spec.

It would be great to have up and down buttons for the Opacity control, though I can see that space is at a premium! Not sure how possible this is. Could an ‘opacity’ icon be used instead of the text? Something like one of these? Happy to work on one for you, if you like this idea.

I really wanted the up/down buttons for that too. I even considered left/right buttons underneath but, yeah, space was too tight. The icon idea is good lateral thinking although I must admit, I’m not sure how well it would fit into Paint’s interface and RISC OS style in general. None of those icons you linked shout “opacity” to me, but if you think you can come up with something intuitive and the community agree, go for it.

The Tab and Return keypresses in the writable icons are incorrect.

Ah that’s interesting. I’ll look into that.

There seems to be a bit of flicker when the brush sprites are selected in the pane. Is there an unnecessary redraw going on?

: ( I must admit I can’t see flicker on my RPCEmu but it doesn’t surprise me. It should only be redrawing the previously and currently selected buttons but maybe I forgot to clip the redraw properly. I did spend a lot of time building the 3D buttons dynamically with vector graphics, which is kind of peverse when sprites would’ve been faster, although they do scale better in different modes and I did have fun!

If possible it’d be nice to have all the settings instantaneous, but from what you said in an earlier post that won’t be easy.

Well basically in the past they all got applied in one big function when OK was clicked, but I understand them a lot better now and most of the work is already done to have everything, except loading by name, instantaneous. It’ll look better with OK next to the Name, though I might change it to “Set” or even “Use” (“Set” does sound like it’s for renaming the brush though, hmmm) and maybe disable it when the text matches the current brush name.

Out of interest, has your new code solved the long standing occasional crash when selecting a brush?

Oh, I really wish I knew! At some point I need to start a thread about that bug. I never yet managed to reproduce the exact exception that you posted about, but I have occasionally had various different exceptions over the years(!-years since starting the bounty!) using the brush that I’ve written down. The commonest seems to be an abort on data transfer. I strongly suspect some of them were my own fault due to my own bugs that I’ve since fixed, but I’m fairly sure one or more do remain.

The very limited diagnostics I’ve managed to do have led me to suspect SpriteExtend as one of the likely culprits, because the most common crashes seem to happen at addresses inside it, when Paint does a Plot Scaled to plot the brush. Unfortunately, the assembler seems to be subtly different every time I look and never seems to match anything I can find in the sources (I suspect it might be some dynamically-generated plot routines that SpriteExtend makes).

I had (and continue to have) tons of trouble with Paint’s translation table stuff, partly because I struggle to fully understand what it’s doing under the skin, and partly because it gets quite complicated when the brush sprite, currently-edited sprite, screen, and now translucent layer, may each need their own translation table! Judging by some of Acorn’s code comments it seems I wasn’t the only one!

I had a one-off error only yesterday where m_FREE() was complaining that some of the memory next to a temporary palette (used to make brush translation tables) didn’t contain a diagnostic value it expected (called JUNK incidentally). This does suggest a possibility of an overflow somewhere causing occasional memory corruption. The trouble is I’ve spent vast amounts of time reading every memory allocation / index / deallocation call in Paint as I come across it and as yet I’ve never found anything I thought was really wrong! I’ve been adding a few sanity checks (with logging) to Paint’s various functions that accept pointers and I think I’ll keep adding more to try and make it more robust. I’m beginning to suspect more and more that it’s bugs elsewhere in the OS that are causing most of these errors, though.

That bug I mentioned where the first plot of the brush on the translucent layer is offset is seriously weird though, which is why I’m trying to make a test program to reproduce it. It’s 100% reproducible inside Paint for me at the moment. I don’t believe I’m doing anything unusual with coordinates on an initial plot, and the mask seems to draw in the right place, so maybe if we can fix this it might resolve something else. At least once I have a cut-down brush test program, I can set it running thousands of times with lots of sprites to start to analyse when and how it does go wrong!

Phew, excuse my rambling!

Another bounty for layers? ;)

That would be wonderful. Unfortunately I seriously doubt I’d be the one coding it though. Time for some new blood… ;)

Chris, one thing I would say, is when you do manage to trigger an error using the brushes, please, please open a task window and run the usual:

*where
*showregs
*memoryi pc -40 +80
*modules

and send me the output.

You can also try enabling tracing in Paint again by adding this line to the !Run file (just underneath “Set Paint$Dir <Obey$Dir>”) :

Set Trace$To f<Paint$Dir>.paintlogfile

and sending me the log file, but be warned it spits out a huge amount of output lately, thanks to me, and slows the program down massively, so don’t forget you left Paint running or it’ll fill up your hard drive!

It’ll look better with OK next to the Name, though I might change it to “Set” or even “Use”

In terms of keeping things simple, does the interface even need a text-input field for the brush sprite now? Dragging to the pane is so much nicer, so I’m not sure whether keeping a writable icon is necessary. Losing it would free up some space, simplify the layout, and possibly enable doing away with the OK button entirely (assuming that you’re able to make the Shape/Scale controls take effect instantly, which I agree would be great). It’ll also make it harder to provoke an error (you can’t type in a sprite name that doesn’t exist).

In terms of keeping things simple, does the interface even need a text-input field for the brush sprite now?

I know you’d dearly love to ditch it. The trouble is, I dearly love to keep it. :D Rick seems to like it too. In its defence, apart from it being fast for those of us preferring keyboard input, there is the additional need for it to distinguish large rectangular sprites that wind up looking almost identical when they’re shrunk onto the buttons. One thing that did occur to me is the name could appear in the text box just above Set/OR/AND/EOR, so it says something like ‘Brush sprite “newsprite”’ instead of “Use sprite as brush” though I fear most people wouldn’t even notice it there-I certainly never look at that box!

I know you’d dearly love to ditch it. The trouble is, I dearly love to keep it. :D

Hehe! It lives to fight another day :)

I’ve just noticed that one advantage of moving the OK button up next to the writable field is that it’ll make room to have up and down arrows for the Scale control. That would also be nice, if it’s on your radar.

The icon idea is good lateral thinking although I must admit, I’m not sure how well it would fit into Paint’s interface and RISC OS style in general. None of those icons you linked shout “opacity” to me

Agreed – they’re not great. Not sure what the best solution here is – it would be nice to have some buttons, but making room for them isn’t easy.

I’ve just noticed that one advantage of moving the OK button up next to the writable field is that it’ll make room to have up and down arrows for the Scale control. That would also be nice, if it’s on your radar.

Yeah, very much so. Hopefully I can squeeze it into this round of changes. If not it might end up being another bonus feature I do later after the bounty is finished.

There seems to be a bit of flicker when the brush sprites are selected in the pane. Is there an unnecessary redraw going on?

Ah, I see now. Paint’s brush OK button code has always forced a redraw of the whole tool window. I guess that was to make it redraw your favourite Name writable icon. I’m not sure even that was necessary though, as the text never changes from what the user entered, even when the sprite didn’t exist. Maybe we can just take that redraw out.

It’s a bit horrible, but I have to wonder if the thinking was that the flicker provided a small visual clue that clicking OK was actually doing something. Without it, it probably looks like the OK button is broken. Now we have a brush pane, the user will see their brush selecting, so that shouldn’t be a problem anymore.

I may be doing something wrong, but I don’t think I’m getting the Shape/Stamp/Tint working correctly for dragged-in sprites.

I created a 16M colour base sprite, then another 16M colour sprite called Brush1. I dragged Brush1 to the pane, where it was selected. I can then paint with this brush, but only in ‘Stamp’ mode. Selecting Shape or Tint, then clicking OK, has no effect. All these settings work correctly with the built-in brush sprites.

I may be doing something wrong, but I don’t think I’m getting the Shape/Stamp/Tint working correctly for dragged-in sprites.

I don’t think you’re doing something wrong. I think my translation table woes are probably continuing. I can reproduce it here. I’m pretty sure it’s the fact the brush sprite has 16M colours rather than the fact it was dragged in. Shape / Stamp / Tint are currently all handled in my ColourTrans transfer function and for some reason ColourTrans seems incredibly tetchy about accepting any changes to the workspace parameters. At least it seems reproducible so hopefully it’s something obvious.

I wonder whether I need to supply the transfer function in a Colour Mapping Descriptor instead of in a translation table for > 256 colour sprites. The documentation is clear as mud about these things.

I think you may have misunderstood what a translation table is.

A pixel translation table is an array with one entry for each source colour. Each entry contains the closest available destination colour for that source colour.

A table translating from the default (not desktop) 4-colour palette to 16M colours would look like this:

int table[4] = { 0x000000, 0x0000FF, 0x00FFFF, 0xFFFFFF }

The same translation, but created with a colour transfer function that converts to grayscale, would look like this:

int table[4] = { 0x000000, 0x3F3F3F, 0xBFBFBF, 0xFFFFFF }

The table does not contain any reference to the transfer function. If you change the transfer function workspace you need to call ColourTrans again to recalculate the table.

A translation table from 16M colours would need 16M entries. That would be ridiculously big, so ColourTrans instead creates a 32K table for mapping down from 16M colours to 256 or fewer colours. In most cases this works well, but the results aren’t great if the destination is 256 grays.

If both the source and the destination have more than 256 colours, then ColourTrans will not create a translation table for you. A full table would be very big, and using a 32K entry table to do colour mapping from 16M to 16M would cause an unacceptable loss of quality. In this case, if you want colour mapping, you must pass a colour mapping descriptor to OS_SpriteOp instead of a translation table.

Thanks yet again Edward!

The table does not contain any reference to the transfer function. If you change the transfer function workspace you need to call ColourTrans again to recalculate the table.

a 32K entry table to do colour mapping from 16M to 16M would cause an unacceptable loss of quality. In this case, if you want colour mapping, you must pass a colour mapping descriptor to OS_SpriteOp instead of a translation table.

I knew the translation tables were lookup tables for the colours but I wrongly thought a transfer function gets used in addition to the table lookup (for rendering) rather than to actually generate the table contents.

A pixel translation table is an array with one entry for each source colour.

Except going to 32K modes where it is a guard word, a pointer to a table held elsewhere, and another guard word, and even more messy going to 16M modes.
There’s a table and description in PRM 5a on page 5a-120 (or p134 in the document).

but I wrongly thought a transfer function gets used in addition to the table lookup (for rendering)

This seems to imply that it can… https://www.riscosopen.org/wiki/documentation/show/Colour%20Mapping%20Descriptor

but I wrongly thought a transfer function gets used in addition to the table lookup (for rendering)

This seems to imply that it can… https://www.riscosopen.org/wiki/documentation/show/Colour%20Mapping%20Descriptor

When calling OS_SpriteOp, you can supply a transfer function or a lookup table, not both.

If you want to render using both a transfer function and a lookup table, you need to supply the transfer function to ColourTrans when it’s generating the table. This will generate a lookup table with the effects of the transfer function “baked in”. The generated table doesn’t contain any references to the original transfer function, so changing the properties of the transfer function after the table’s been generated will have no effect on that table.

I think there is one edge case to be aware of, which is that sometimes ColourTrans will decide not to return a translation table (e.g. if the source and dest are identical modes & palettes, or the source and dest are both true-colour), and that decision is made without looking at the transfer function. So if you pass a transfer function into ColourTrans, and ColourTrans doesn’t return a translation table, then you’ll have to instead pass the transfer function into each OS_SpriteOp call.

Here’s a new test build for the Brush Tool Enhancements.

As before:

It’s an ArcFS archive. The !Paint program is inside Paint.debug, built for IOMD (Risc PC / RPCEmu) but you can build from source if you want to. My code’s still a little messy as I haven’t ironed out all the issues yet.

• I’ve fixed the issue where Shape / Stamp / Tint wasn’t working correctly with > 256 colour sprites (a Colour Mapping Descriptor is now used when necessary).
• There should now be less flicker over the tools window when selecting a different brush (it no longer forces a redraw of the whole window).
• Previously you could drag the mouse past the end of the opacity slider bar, making it hard to select full Opacity. This is now fixed.
  

By the way, sorry for putting these builds on a site that needs Javascript for you to download them!

sorry for putting these builds on a site that needs Javascript for you to download them!

Oh my god.

Firefox that makes it easy to look at every fetch that was made, so it’s usually possible to extract the actual download links (useful for Github, Drive, etc).
This one is https://public.boxcloud.com/d/1/[...]./download where the ‘…’ is about a thousand characters of random gibberish that probably encodes the filename, owner, your IP address, the time, the time in Martian, the Julian date, the phase of the moon, and the current number of people known to be infected by the modern plague.
Jeez.

I just got brush painting with the transparent colour working again.

I’ve noticed one downside of making use of blend tables / inverse tables for brush opacity. Draw some transparent strokes on the image. Then choose a solid colour and, with Opacity turned down, paint bands of it across the transparent strokes. When the temporary layer is merged down, the intersecting areas are the wrong colour. It’s obviously blending with the RGB colour of the bottom sprite, ignoring the fact that those pixels are masked as transparent. I don’t think there’s any easy way around that. To minimise the effect it might be worth seeing if we can paint the RGB colour in a neutral grey when painting areas with the transparent colour (at the moment Paint doesn’t care what colour it ends up).