bugMyPaint - Bugs: bug #19044, Ignoring "Eraser"...

 
 
Show feedback again

You are not allowed to post comments on this tracker with your current authentification level.

bug #19044: Ignoring "Eraser" brushes for certain actions

Submitted by:  Till Hartmann <tillux>
Submitted on:  Tue 22 Nov 2011 09:14:00 PM UTC  
 
Severity: 2 - MinorPriority: 5 - Normal
Status: FixedPrivacy: Public
Assigned to: NoneOpen/Closed: Closed
Release: Planned Release: None
Operating System: 

(Jump to the original submission Jump to the original submission)

Fri 04 Jan 2013 06:13:35 PM UTC, comment #13:

This bug is already Fixed in git, but is marked as Open. I am now
marking it Closed because a new stable version, 1.1.0, is
available, meaning that this bug no longer affects the current
stable release.

Please reopen if this problem reoccurs with the new version of
MyPaint!

Andrew Chadwick <achadwick>
Project Administrator
Wed 25 Jan 2012 10:48:53 PM UTC, comment #12:

I pushed a fix: https://gitorious.org/mypaint/mypaint/commit/38ada522fd

That should fix both problem 1) and 2), even if only for new ORA files.

I don't think we need a "pickeable" brushsetting, I think the pixops.hpp code handles dynamic erasers/smudging quite well, and had nothing to do with the problem.

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 07:08:53 PM UTC, comment #11:

Hm... actually, what you propose in your patch - simply not recording eraser brushes into the strokemap - might be a very pragmatical "good enough" solution, if we want to keep the "combined" undo steps...

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 07:00:21 PM UTC, comment #10:

I figured out how to reproduce. Select a normal brush, toggle it into eraser mode, then, in mid-stroke, toggle it to normal mode.

This generates a single undo step that does both erase and paint. When you pick the paint, you get an eraser brush.

In some earlier MyPaint version, we had a stroke split for every brush change. There was even a bugreport, because changing the size with the keyboard did trash the undo stack then. While it makes some sense not to split an undoable stroke for every radius change or opacity, we really should split when toggling eraser mode.

At the moment, we don't even split an undoable stroke when you change the brush. So if you start a brush with brush A, then use a brushkey to switch to brush B, the strokemap will record only brush A.

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 06:29:06 PM UTC, comment #9:

The code tries to estimate the "amount of perceptual change". It does not only ignore erasers, it also ignores very faint strokes. It ignores any stroke that makes little difference (eg. painting with red on red).

The problem dropping eraser strokemaps based on .is_eraser() is that brushes can change their settings dynamically. But maybe we could ignore strokemaps that are empty (which should be true for all erasers).

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 03:38:07 PM UTC, comment #8:

> I think it would be much better to fix that, rather than add user-visible settings for something that is supposed to "just work". I doubt that anyone would really want to tune this brush-by-brush.

It's not only about eraser brushes, but also about blend/smudge brushes (if that makes any difference at all)

> About the last active stroke: we could simply walk through the list of most recent strokes, and examine their brush settings, until we find one that is no eraser.

That's what I first did until Andrew pointed out not-adding eraser-strokes to the stroke-map at all might work better.

> So far I have never seen an eraser brush generate a strokemap

me neither, but haven't had the time to test thoroughly yet.

> In pixops.hpp line 701 there already is code to ignore erasers. Does it not work?

Didn't know that, but it obviously doesn't quite work as it should (or something entirely else fails)

Till Hartmann <tillux>
Project Member
Fri 13 Jan 2012 09:30:08 AM UTC, comment #7:

I pushed some debug code to show the recorded strokemap as strokes are made: https://gitorious.org/~maxy/mypaint/maxy-experimental/commits/strokemap_debug

So far I have never seen an eraser brush generate a strokemap, so maybe we store the wrong brushinfo into another stroke? Or maybe something goes wrong when selecting the brush that was restored?

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 08:59:01 AM UTC, comment #6:

Hm... in general ignoring erasers seems to work, but I just loaded an painting I did, and noticed that I could pick an eraser brush from it (allthough I suspect that the stroke that is highlighted was not actually an eraser stroke).

Do you know a way to reproduce the problem?

Martin Renold <martinxyz>
Project Administrator
Fri 13 Jan 2012 08:36:56 AM UTC, comment #5:

In pixops.hpp line 701 there already is code to ignore erasers. Does it not work?

I think it would be much better to fix that, rather than add user-visible settings for something that is supposed to "just work". I doubt that anyone would really want to tune this brush-by-brush.

About the last active stroke: we could simply walk through the list of most recent strokes, and examine their brush settings, until we find one that is no eraser.

Martin Renold <martinxyz>
Project Administrator
Thu 12 Jan 2012 10:37:38 AM UTC, comment #4:

Yep, obviously forgot 'not'.
Stroke sounds like a good place to put it in, renaming to stroke_pickable would only be logical then.
Not sure about whether to have this as a visible setting or not, as it won't change often (per brush). For existing brushes, this doesn't make a difference, we'd just have to adjust them once, but what about brushes created from scratch?

OT: What about making brushsettings have a 'boolean' flag to make it possible to display checkboxes instead of sliders? Probably not worth the effort and the loss in simplicity (codewise). And not even the win in UI clarity, as most users won't ever see this. Ah well, doesn't belong here.

Till Hartmann <tillux>
Project Member
Thu 12 Jan 2012 02:11:10 AM UTC, comment #3:

Looks good so far.

I think it should go in Stroke and not Custom, since there's a setting in Stroke for the input pressure threshold already. Maybe even call it stroke_pickable?

Also, is this a bug, or am I just misreading it?

"or not self.is_eraser()", surely?

Not sure if any UI is needed for this. Somebody'll need to do a quick review of the built-in brushes and add it in where appropriate though.

Andrew Chadwick <achadwick>
Project Administrator
Thu 05 Jan 2012 06:37:44 PM UTC, comment #2:

Here's a patch which does the following:
Add a brushsetting "pickable". This defaults to 1.0; which means by default all brushes are pickable. However, this does not apply to erasers, which are generally ignored (read: "not added to the strokemap in any case").
As Andrew said, it'd make sense to make smudge and blending brushes not-pickable either (though I'm not sure what the exact criteria might be, as there are for example some blend+paint brushes in the default brushset[s]).
Uh, I added the setting to the "custom" Group, as that was a rather convenient place (for testing). Might make more sense to add a checkbox like the realtime-update-last-stroke one.

(file #14764)

Till Hartmann <tillux>
Project Member
Thu 05 Jan 2012 11:44:09 AM UTC, comment #1:

As mentioned in bug #19240 - this could equally well apply to smudgers, so perhaps a per-brush setting for this?

Andrew Chadwick <achadwick>
Project Administrator
Tue 22 Nov 2011 09:14:00 PM UTC, original submission:

1) pick-context should not pick erasers
2) last-active-stroke (i.e. mypaint uses the last-active-stroke as the startup-brush(-setting) for its next launch) should ignore erasers

Till Hartmann <tillux>
Project Member

 

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by martinxyz (Posted a comment)
  • -unavailable- added by achadwick (Posted a comment)
  • -unavailable- added by tillux (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 04 Jan 2013 06:13:35 PM UTCachadwickOpen/ClosedOpen=>Closed
    Wed 25 Jan 2012 10:48:53 PM UTCmartinxyzAssigned totillux=>None
    Wed 25 Jan 2012 10:48:52 PM UTCmartinxyzStatusConfirmed=>Fixed
    Fri 13 Jan 2012 08:59:01 AM UTCmartinxyzStatusReady For Test=>Confirmed
    Thu 05 Jan 2012 06:37:44 PM UTCtilluxAttached File-=>Added 0001-pick-context-ignore-erasers.patch, #14764
      StatusIn Progress=>Ready For Test
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup