Implement Masks - Githubissues

weisJ commented 3 years ago

This is a somewhat rudimentary implementation of masks. Most likely it can be done in a more efficient manner, but compared to no support for masks at all this is probably still better.

I have tested the implementation using the examples from https://github.com/blackears/svgSalamander/issues/66. However there might still be some edge cases which aren't properly handled and it certainly isn't a spec compliant implementation. For example it is always assumed that the color-interpolation attribute is linearRGB and masUnits are ignored.

For the picking mechanism I had to make some trade-offs:

If boundingBox is specified then the mask is ignored. I am not sure how the mask would interact in this mode so I decided that it simply shouldn't count.
If a pick area is given then the mask will only block the picking operation if there isn't at least one pixel that is partially visible. This might lead to some unexpected results where an element clips the masking area where the element isn't shown, but the mask doesn't fully hide the picking area, hence the element gets included in the pick result. (these are probably rather niche cases)
Also for picking areas if the given AffineTransform has a rotational or shear component then the picking result will most likely be incorrect.

Nonetheless the current state of picking works as expected with the examples from the issue.

weisJ commented 3 years ago

Somehow the diff for two files is quite large. I’ll need to fix this first. But other than that this PR is complete (from my point of view)

weisJ commented 3 years ago

Looks like Group.java and RenderableElement.java are commited with CRLF line endigs. What a pain! If allowed I'll provide a PR with a .gitconfig file to prevent this from happening again.

weisJ commented 3 years ago

Would it be possible to cache the Mask object for the element in the build method? I’m not sure whether this would break any guarantees about reflecting runtime changes to the svg structure.

blackears commented 3 years ago

I've not had the time to review your changes yet. It looks like quite a substantial addition - thanks! But I've not started to review it yet due to dealing with other work. I hope to get back to you soon about this.

blackears commented 3 years ago

Hi. I was just reviewing the code. I noticed that you changed render() to doRender() in many classes and also make it protected instead of public. Same for pick()/doPick(). Is there a reason for this? Changing these methods could break a lot of third party code that relies on them.

weisJ commented 3 years ago

render and pick are still available. If an element doesn’t have a mask attached the simple delegate to the doRender and doPick methods respectively. So for any outside person calling them there won’t be any visible change as the api surface stays the same. This change was necessary as masked painting and picking needed to wrap the existing render/pick calls.

blackears commented 3 years ago

I can merge this branch now. Please let me know if you are finished with changes.

weisJ commented 3 years ago

~~I am finished with changes.~~ I just wanted to add the mask caching. Retrieving the Mask every time render or pick is called was unnecessary.

Edit: There is an issue with drawing the SVG with a rotated Graphics transform. I'll have to fix this before.

weisJ commented 3 years ago

Transformed Graphics now work properly.

blackears / svgSalamander

Implement Masks #78