Bounty proposal: Paint

If you were building with a “Disc” environment then it’s a bit odd to be getting BLX instructions in the executable.

• Did you switch environment without cleaning the !Paint sources?
• What build directory are you using – had you previously used it to build something with a non-Disc environment? Paint uses the RISC_OSLib library, so it could be an incompatible build of that which is causing the problems.

If you were building with a “Disc” environment then it’s a bit odd to be getting BLX instructions in the executable.

Did you switch environment without cleaning the !Paint sources?
What build directory are you using – had you previously used it to build something with a non-Disc environment? Paint uses the RISC_OSLib library, so it could be an incompatible build of that which is causing the problems.

I must confess, once I got a working build on RPCEmu I stopped paying attention to my build environment (on the grounds that this could be adjusted later if necessary) and I honestly can’t remember exactly where I copied my source tree from initially but the top level directory is called IOMDHALDev (I suppose because I initally wanted to build the code for RPCEmu and my Risc PC) and that’s what I currently select for my build directory, then choosing ROOL.Disc as the build environment. I’m probably doing it horribly wrong. I guess there’s a “Disc” build tree that I should be using, right?

I guess there’s a “Disc” build tree that I should be using, right?

Oh dear, I was using a Titanium environment, I don’t have DiscDev. How is one supposed to know about that.

Oh dear, I was using a Titanium environment, I don’t have DiscDev. How is one supposed to know about that.

By reading the exceedingly clear docume…

Oh, well. Let’s say by psychic powers. :)

That download is identical to the previous 2 as verified by !DirSync,

That suggests either ArcFS or more likely the file hosting website are being a pain!

I’ve cleaned and rebuilt with a completely new archive Filename PaintBounty19Feb17_1

That suggests either ArcFS or more likely the file hosting website are being a pain!

Or some cache in the web pipeline. Best to use a new filename and link each time to force the issue.

Filename PaintBounty19Feb17_1

Got that, and a DiscDev environment. A build now doesn’t crash on the A9home, and zooms incrementally.

The bad news is that the zooming is still wrong on the Titanium. I have now got the updated source in c.SprWindow.

*show USBNoScrollFix
USBNoScrollFix : 0
*

OK, somewhat late to the party, I’ve had a play on both RPCEmu and the Pi. Great you’re working on this, Andy :)

Spray can comments:
- I’d make the default density something like 100, rather than 20. Otherwise, as David says, it feels a bit sluggish to be useful.
- Can you add increment and decrement arrow icons to the tool window pane? That would make it easier to adjust the values without reaching for the keyboard, RISC OS 2-style :)
- The redraw is pretty flickery, although no worse than the original Paint. This is a pretty minor point, but I wonder if there’s anything that could make it less so?

Scrollwheel scroll/zoom
- To be honest, scrolling around the window by a large interval doesn’t bother me. Could Shift-scroll, or Alt-scroll, perhaps move the image horizontally too? That would make scrolling around big sprites really nice.
- The scrollwheel zoom is more of a challenge, leaping from x1 to x4 in one go. This would be easier to live with if the sprite window didn’t change size, since when editing a big sprite even one ‘click’ of the scroll wheel fills the monitor. My suggestion would be to make the default action to zoom in/out of the sprite magnification while keeping the window the same size on screen (unless zooming out forced the image smaller) – what do you think?

the top level directory is called IOMDHALDev

That should be OK shouldn’t it, it’s old stuff, should be any newer instructions in builds?

Using a Titanium environment for use elsewhere is clearly asking for it!

A Basil Fawlty moment perhaps.

P.S.
Processor flags are defined in castle.RiscOS.Env.!Common/sh

These are called by Obey files in castle.RiscOS.Env.ROOL

The bad news is that the zooming is still wrong on the Titanium. I have now got the updated source in c.SprWindow.

That’s good at least that USBNoScrollFix is 0, that’s what it’s supposed to be on the Titanium. Looking over my own code, I suspect I’ve overlooked a break statement :(

      else if (e->data.scroll.y == 1)
           {
             /* Zoom in */
             if (window->data->sprite.blobsize.scale_xdiv > 1)
               window->data->sprite.blobsize.scale_xdiv--;
             else if (window->data->sprite.blobsize.scale_xmul < 999)
               window->data->sprite.blobsize.scale_xmul++;
             menus_showmag (&window->data->sprite);
           }
           break;

The break statement means that when the zoom happens, my event counter never gets incremented. Still, at least these are turning into productive test sessions I suppose. Thanks for your help everyone!

OK, somewhat late to the party, I’ve had a play on both RPCEmu and the Pi. Great you’re working on this, Andy :)

Thanks again for your help Chris!

- I’d make the default density something like 100, rather than 20. Otherwise, as David says, it feels a bit sluggish to be useful.

Yeah, I’ll probably bump up my speed multiplier a bit and/or change the default density as you say. It’s a bit of trial and error as the old rate was very machine dependent of course.

- Can you add increment and decrement arrow icons to the tool window pane? That would make it easier to adjust the values without reaching for the keyboard, RISC OS 2-style :)

Maybe, if I have time next time I’m working on the templates.

- The redraw is pretty flickery, although no worse than the original Paint. This is a pretty minor point, but I wonder if there’s anything that could make it less so?

Yeah, I dislike the flickeriness also. On RPCEmu I see a sort of flickering black rectangle but I assumed this was an emulation issue. Do you see it on the Pi also? I don’t know whether Paint waits for VSyncs, though the spray algorithm is pretty slow. :(

Scrollwheel scroll/zoom
- To be honest, scrolling around the window by a large interval doesn’t bother me. Could Shift-scroll, or Alt-scroll, perhaps move the image horizontally too? That would make scrolling around big sprites really nice.

That’s a good idea that should be fairly straightforward to add.

- The scrollwheel zoom is more of a challenge, leaping from x1 to x4 in one go. This would be easier to live with if the sprite window didn’t change size, since when editing a big sprite even one ‘click’ of the scroll wheel fills the monitor. My suggestion would be to make the default action to zoom in/out of the sprite magnification while keeping the window the same size on screen (unless zooming out forced the image smaller) – what do you think?

The big steps are a definite bug, as discussed. I’m not hugely keen on resizing the window though. Don’t forget one of the other features is to be an auto zoom on Ctrl+Resize.

@David

the top level directory is called IOMDHALDev

That should be OK shouldn’t it, it’s old stuff, should be any newer instructions in builds?

That’s exactly what I thought. Was it my build (included inside the “debug” directory) that had the undefined instruction, or were you always rebuilding from source?

Famous last words, but this should, OK, might fix the scroll zoom bug now.

I’ve also added support for horizontal scrolling of the window with Shift+mousewheel as requested by Chris. Amusingly at the moment Shift+click on the vertical scroll buttons will do that also at the mo, as Fred pointed out. It won’t stay that way. It’s a shame I can’t respond directly to the mousewheel vector or that it didn’t have it’s own additional event.

On RPCEmu I see a sort of flickering black rectangle but I assumed this was an emulation issue. Do you see it on the Pi also?

Yep, a bit, and obviously more noticeable with larger radiuses and a masked sprite. The key issue, I suppose, whether it’s worth spending time fixing this – everything works well, and if it requires delving into Paint’s redraw system to fix then it might not be worth the trouble. Since the spraycan is used as a ‘hold down Select for a long time’ kind of tool, one possible optimisation might be to only update the scaled counterpart sprite in the Sprite file window once a button press is released, rather than at every iteration of the spray?

I’ve also added support for horizontal scrolling of the window with Shift+mousewheel as requested by Chris.

Great! Really helpful :) Is the scroll direction correct though? I’d have thought a ‘down’ scroll would send the scrollbar right, and an ‘up’ scroll left.

I’m not hugely keen on resizing the window though.

Fair enough :) With the x5 bug fixed, the zoom’s a really useful new addition.

Great! Really helpful :) Is the scroll direction correct though? I’d have thought a ‘down’ scroll would send the scrollbar right, and an ‘up’ scroll left.

Agreed! Laziness on my part just trying to get the feature into the build as quickly as possible. ;) I’ll fix it later.

With the x5 bug fixed

Wow is it definitely working now then?

With the x5 bug fixed

Wow is it definitely working now then?

Yes, both zoom and h-scroll, as tested on the Titanium & A9home.

Yes, both zoom and h-scroll, as tested on the Titanium & A9home.

Woohoo! We got there in the end. Thanks. It’s always a bit of a shot in the dark when I can’t test my own code locally.

The downside is that the scroll arrows only work every fifth press.

The downside is that the scroll arrows only work every fifth press.

Yeah that’s the issue that Fred pointed out. It needs some logic to differentiate mouse wheel events from scroll button clicks, as the scroll event is identical. Elsewhere in the forums someone suggested using the mouse pointer location – which is not completely infallible as it can’t differentiate between hovering over the scroll buttons and clicking them – also clicking and then rapidly moving away won’t work – but it should be good enough.

Elsewhere in the forums someone suggested using the mouse pointer location

That would be my idea as well. Call Wimp_GetPointerInfo in your scroll handler, check that either Select or Adjust is pressed and that the icon handle is -6 (up arrow) or -8 (down arrow). And perhaps, as an extra check, make sure the window handle matches that of the scroll event.

There may be some corner cases where this doesn’t work but generally it should be fine.

Apologies if this is deemed hijacking the thread but I’ve recently been playing with scroll events from the wimp.
If I click a bump arrow, i get a +1 or -1
If i click on the slider either side of the scroll bar I get a +2 or -2
I can then re open the window after adjusting the scroll offsets by whatever amount i want.

IF i use the scroll wheel on the mouse I get 5 consecutive calls to my code. If a add a value to the scroll offset ie by 1 line amount every time i enter my function it will only adjust the window by 1 single line, not 5 like i would expect.
So why doesn’t the window scroll offset get updated for the first 4 calls ?

That looks like another issue with the USB driver; it reads the window state once and then uses that same state for all five calls to Wimp_SendMessage. Considering that this is done in an interrupt handler, I’m guessing the Wimp will queue the messages, so even if it did read back the window state after each scroll request it wouldn’t see any difference.

Also,

b) if a window can accept scroll requests, send them regardless of whether the window has scroll bars (some applications might like this hint hint).

The original scroll implementation in the USB driver didn’t check to see if scroll bars were present. But then when I added support for horizontal scroll wheels it was found that you could scroll filer windows sideways and end up with a silly display.

That looks like another issue with the USB driver; it reads the window state once and then uses that same state for all five calls to Wimp_SendMessage. Considering that this is done in an interrupt handler, I’m guessing the Wimp will queue the messages, so even if it did read back the window state after each scroll request it wouldn’t see any difference.

Aha, i thought it was doing something odd. That makes sense now.

Just tried out the new spray can tool. Works great here – well done!

Thanks Chris! This bounty’s progressing a bit slower than I would have liked, so thanks everyone for your patience! Like many of you I have lots of things demanding my attention but I tend to have big bursts of productivity so it’ll get there.