Refactor ImageView drawing to remove checker background from render#685
Refactor ImageView drawing to remove checker background from render#685HiCamino wants to merge 1 commit into
Conversation
24059d6 to
1f79d69
Compare
|
this was allot of work and a headache and if it all works as it should you should not see any difference at all :( but it makes the code better and more consistent with haiku and also removes the checker bg from the rendered image so #674 will work also it looks better when you use manipulators or move the whole canvas bc the checker bg is not part of the move anymore |
|
Appears to work nicely! Very well done! Leaving those PRs open for a bit to allow more feedback.
There's one difference: The checker-bg doesn't appear fixed when zooming in anymore, as the checker-size is fixed, where it used to increase with the zoom level. At first I was a bit distracted, because the checker-bg appears to be moving in relation to the actual pixels of the image. This is only really noticable when using the mouse-wheel. |
oh i forgot to mention this. I am on the fence if its a good thing or bad thing because if you have a big image then the checker squares are very small and so having the squares stay the same no matter the zoom it sort of makes sense but if you really dont like it the fix is easy to use DrawBitmap instead of DrawTiledBitmap and make the bg bitmap the size of the whole image now its just two squares that get tiled for efficiency of memroy and drawing |
|
For very large images, if you work much with a low zoom-level, people can change the checker size in the settings, if it's really annoying. Over all, I think I prefer the 'old way', i.e. resize the checkers when zooming. It's much less distracting when using the mouse-wheel. Plus, with the checkers staying fixed in relation to the image-pixels, you can use the checkers to keep track of a particular location of an image. I'm having trouble explaining this... :) You can memorize where the ellipse intersects with a background checker (the black X above) and keep your eyes trained on it while changing zoom. If the checker-bg changes WRT to the image pixels you lose that. |
- Set default for "bg" parameter in Image::Render to false so it doesn't draw the checker bg - Remove code to render the checker bg in Image::DoRenderPreview - In ImageView add checker bg bitmap. it is the size of the image and is drawn as a bitmap first thing in the ImageView::Draw function. the bitmap is drawn in the ImageView::MakeBackground() function and this is also called when the settings change to redraw the background bitmap - in ImageView::Draw the background is drawn first and the brush drawing was also moved here. this is because in Haiku drawing should happen only in the Draw() function so other Draw commands were removed unless in functions called by Draw(). - ImageView::BlitImage() uses alpha drawing mode to draw the changed bitmap parts - ImageView::UpdateImage() uses Invalidate() to force redraw but doesn't use BlitImage() anymore - ImageView::MouseDown() invalidates window region when scrolling view to force redraw - ImageView::MouseUp() code is not needed because DrawBrush happens in Draw() - ImageView::MouseMoved() invalidates area where brush is drawn but doesn't draw brush because it happens in Draw() function. Also the Draw() when the mouse leaves the view is changed to Invalidate() - ImageView::DrawBrush() clear code is not needed because Draw erases the bg before calling DrawBrush. Also calls to Draw are replaced by Invalidate() - A few tools (FreeLineTool, EraserTool, HairyBrushTool) make sure updated_rect is within the image bounds to prevent crashes NOTE: with this commit, Selection and Color Selector have artifacts. These will be fixed in separete commits
1f79d69 to
655749d
Compare
|
ok i made the bg static like before |
655749d to
780dd24
Compare
|
i found an issue that needs to be fixed. the crop rectangle doesnt draw on top of the background anymore |
780dd24 to
07cd12e
Compare
|
fixed drawing outside of image for crop etc. also it doesn't flicker so its better then current ArtPaint |
|
LGTM, after playing around a bit. Hope we've tested enough, the app is quite complex... :) When rebasing #686 I had to resolve some conflicts. Maybe have a look before the rebasing starts. (Though I admit I'm not too good juggling branches...) |
|
Seeing as their commit message title is very similar, it'd be a good idea to sqash them all into one, if you don't mind. I suggest we merge this weekend, if there are no objections from anyone. |
|
ok you should only need to merge #687 bc it should have all of the changes and then we can close 685 and 686 |
|
Superceded by #687 |

NOTE: with this commit, Selection and Color Selector have artifacts. These will be fixed in separete commits
Color Selector fix is #686
Selection tool fix is #687
please either test it all together, or if this is tested separate then ignore color selector or selection tool artifacts