Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rectangle/Ellipse: Add "Outline width" slider #629

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

dsizzle
Copy link
Collaborator

@dsizzle dsizzle commented Oct 2, 2023

Addresses #174

... I'm not sure about the "Adjustable outline width" option. Would that happen after rotation? I didn't add that part for now.

A few caveats; the preview drawing has some issues where it doesn't draw correctly, but the actual drawn shape is usually correct. For example:

  1. With large widths, a rotating rectangle gets the corners cut off, but when you click to accept it, the rectangle draws properly with no cutoff corners.
  2. With large widths and small rectangles, there's a diamond-shape drawn in the center - seems like maybe a bug with Haiku's PenSize, where if it's larger than the shape you're drawing there are artifacts?

(1) might also be weird Haiku view drawing behavior?

@humdingerb
Copy link
Member

Very appreciated change!
Can something be done against the flickering while rotating?

I'm not sure about the "Adjustable outline width" option. Would that happen after rotation? I didn't add that part for now.

Personally, I'd rotate first, then adjust the width. But is doesn't really matter...

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 2, 2023

Can something be done against the flickering while rotating?

well...since it's code I'm sure there is something that can be done. 😄 But I would need to learn what that is. The problem is just that we need to draw the shape and erase it and re-draw it and I'm not totally sure how to do that with a BView. I believe I've seen stuff with "offscreen" views or bitmaps in Haiku. This flickering occurs pretty much everywhere there's an interactive UI in ArtPaint but I think it's just a lot more noticeable with thick solid shapes. The easiest "solution" in this case would be to not show the line thickness until after you are done rotating, but that seems less than ideal.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 2, 2023

This flickering occurs pretty much everywhere there's an interactive UI in ArtPaint but I think it's just a lot more noticeable with thick solid shapes.

Hmm, I lied, the straight line tool with a Width of 100 doesn't flicker at all. I will look into it.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 2, 2023

I think I made it a tiny bit better, but to really fix it the way the drawing is done would need to be changed to the way StraightLineTool does it - which to render it the same way we render the final straight line, using ArtPaint's drawing methods. Rectangle and Ellipse use Haiku's drawing. I may attempt this.

@humdingerb
Copy link
Member

Good luck on your anti-flicker mission!
BTW, would it be possible to have the cursor show the outline-width before clicking in the canvas to create the rect/ellipse? Like the brush-size-outline for the other tools.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 3, 2023

Good luck on your anti-flicker mission!

It’s actually going pretty well. I might be done by tomorrow. The code is rather ugly though.

BTW, would it be possible to have the cursor show the outline-width before clicking in the canvas to create the rect/ellipse?

I’m not sure how to do this so that it wouldn’t be confusing. Like just a circle of the appropriate size?

@humdingerb
Copy link
Member

Ugly but working code may be improved with time. :)

I’m not sure how to do this so that it wouldn’t be confusing. Like just a circle of the appropriate size?

Yes, I think a circle or square of the outline-width would give an indication what to expect.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 3, 2023

Yes, I think a circle or square of the outline-width would give an indication what to expect.

what about when “fill” is checked? Don’t show it?

@humdingerb
Copy link
Member

Don't show it, I'd say, because a filled rect has no outline.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 3, 2023

I feel like that’s a little weird because then sometimes you see it and sometimes you don’t?

@humdingerb
Copy link
Member

Your call, ofc. Not confusing to me though 😃
The cursor shows the outline width. Filled rect has no outline, therefore no cursor-size-preview.

@dsizzle dsizzle force-pushed the rect-ell-width branch 2 times, most recently from 9f79a17 to 83f0d65 Compare October 3, 2023 22:40
@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 3, 2023

Ok - please give it a try when you have a moment - there are some artifacts when you rotate quickly or resize quickly but I think it works well overall. I even think it works when you zoom while drawing/rotating.

Haven't tried the draw-circle-to-indicate-size yet; this is just the flicker fix.

@humdingerb
Copy link
Member

Yep, appart from the artifacts, the heavy flicker is gone. Nice!

- Changed Rectangle and Ellipse drawing to use multiple buffers like
other drawing tools. This allows display of actual alpha color as well
as new outline width feature.
@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 4, 2023

How bad are the artifacts on your end?

@humdingerb
Copy link
Member

humdingerb commented Oct 4, 2023

Quite bad actually, if I move the mouse quickly. See [MPG 2.3 MiB]:

rect_arti.mp4

(At the end I zoom in and out to show these artifacts are actually pixels on the canvas.)
(Hope this shows or can be downloaded.)

A still image:

rect_arti_still

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 4, 2023

I tried both in WebPositive and Firefox on Linux and the video doesn't show - but the image is enough. It is bad that the artifacts actually paint on the image permanently.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 4, 2023

updated and I think it's better - I can only see slight corner artifacts on rotation of a rectangle, and I can't get them to stay. But the artifacts when resizing should be gone now.

@humdingerb
Copy link
Member

I tried both in WebPositive and Firefox on Linux and the video doesn't show

Sorry about that. I tried uploading it to my ussual https://0x0.st, but it kept timing out.
So I uploaded to github, hoping it were possible to download the clip from here, if playback in the browser didn't work...

Anyway, with your latest change it is much better. Resizing, as you said, doesn't leave artifacts anymore at all. But I still manage to produce some sticky artifacts when rotating. Most disappear when I keep rotating, but a few remain.

There's also quite some tearing, isn't there?
As I may have touch on a related topic recently in one of my famous one-line contributions. Is BScreen's WaitForRetrace() something that could help with flicker/tearing?

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 6, 2023

Ok, I made the rotating a bit more efficient and shouldn't have any artifacts. Occasionally I see cut-off corners but no extra artifacts, and the corners always reappear when the rectangle is final.

I tried to address the tearing by slowing down the updates and removing the forced image updates and I think it's better. If you are resizing like a crazy person you can't expect a perfect view. 😆

@humdingerb
Copy link
Member

humdingerb commented Oct 6, 2023

Much better!

And yes, the cut-offs seem to depend on rect size and possibly where it is on the canvas. Only apppear whilst rotating and are gone when actually drawing the result on canvas.

A video uploaded to my mailbox.org cloud. Let me know if you can view/download it.

rect_cut-off.avi

Otherwise, merge when ready, I'd say.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 6, 2023

Ok, I was able to download and watch the video - that's far worse than what I was referring to. I have only seen tiny corners getting cut off, not like 1/3 of the rectangle. I'll see if I can repro and fix that before merging.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 6, 2023

ok, I think I fixed it.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 7, 2023

No, NOW I think it's fixed for real. 😄

@humdingerb
Copy link
Member

Very good!
Found #633, unrelated to thiis change though.

@dsizzle dsizzle merged commit f9e9d4d into HaikuArchives:master Oct 7, 2023
1 of 2 checks passed
@dsizzle dsizzle deleted the rect-ell-width branch October 7, 2023 14:17
@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 7, 2023

merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants