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

Properly design the "filters" tool #308

Closed
11 of 12 tasks
maoschanz opened this issue Dec 7, 2020 · 0 comments
Closed
11 of 12 tasks

Properly design the "filters" tool #308

maoschanz opened this issue Dec 7, 2020 · 0 comments
Labels
enhancement ideas to improve existing features tools bugs or suggestions related to tools
Milestone

Comments

@maoschanz
Copy link
Owner

maoschanz commented Dec 7, 2020

Use case

The code is ugly, not modular, and the UI is a manual repetition of widgets. It makes filters hard to implement or improve, and probably not very cool to use

Suggested solution

yet another state pattern:

  • ToolFilters should have a simpler operation dict, with a filter_id, most other attributes would be there depending on the filter used;
  • ToolFilters would have a dict which associates each filter_id to an implementation of AbstractFilter:
    • write AbstractFilter, which has at least a method that will be called in build_operation, and one in do_tool_operation;
    • implementation for blur
    • implementation for transparency
    • implementation for contrast
    • implementation for saturation & veil
    • implementation for "invert colors"
      • it would be easy to add dozens of filters with just [operator]+[color]+[context.paint()]
    • [ ] it would be easy to add a "sharpen" effect with some combination of blur and contrast it isn't
    • it would be easy to add an "emboss" effect with some combination of blur, invert and transparency
    • [ ] it would be easy to add a "glow" effect with some combination of blur and transparency it is, but it's not as useful as expected
  • menus would still be manually written, so each filter might corresponds to as many items as pertinent (+ they can define options, like the blur direction)
  • the option pane would be built with methods from each filter,
    • either with a method packing anything needed,
    • or, assuming a spinbtn is enough, a method setting its adjustment/tooltip/label/icon/visibility.

Possible drawbacks

it should stay responsive and simple...

the biggest drawback imo is that it's not really modular with manually built menus

@maoschanz maoschanz added enhancement ideas to improve existing features tools bugs or suggestions related to tools labels Dec 7, 2020
@maoschanz maoschanz added this to the 0.8 milestone Dec 7, 2020
maoschanz added a commit that referenced this issue Dec 11, 2020
maoschanz added a commit that referenced this issue Dec 11, 2020
This is the main part of issue #308, with several architectural goals in mind:
- the bottom pane implementation should be far more independent from the tool
- the "filters" tool should use lighter dicts in the history
- both should not implement dozens of distinct behaviors

This commit fix that issue, and it should simplify the readability of that part of the app, and ease the development of future new filters.

As explained in the issue details, the menus are still written manually, independently of the exact set of filters loaded by the tool
maoschanz added a commit that referenced this issue Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ideas to improve existing features tools bugs or suggestions related to tools
Projects
None yet
Development

No branches or pull requests

1 participant