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
Clip! Clip! Clip! #8
Conversation
…er clipping to grammar and semantics
Maybe make an issue for the outer clipping of custom polygons? We are then (almost) guaranteed to not forget about it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things to consider to the future but for now, everything looks great!
return self.clipped_in | ||
|
||
def is_outer(self): | ||
return not self.clipped_in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a new line to the end of the file
@@ -50,6 +51,8 @@ def render(self) -> str: | |||
s += ' ' + self.stroke.render() | |||
if self.fill != None: | |||
s += ' ' + self.fill.render() | |||
else: | |||
s += ' ' + 'fill="#000000" fill-opacity="0.0"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe for such defaults we should have a defaults.py file with a bunch of enums and default values. So, if we decide to change something later, it will be much easier to hunt down all the cases where it was used. For example if we want to change the default color.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a separate PR though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sounds like a good idea for other default values. I only put black in there because it needed a fill color. All this does is make the fill invisible when PhoTeX is supplied "no fill" for a shape!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but what if in the future we want the default color to be customization or we want it to be white? We would need to go through all the files and manually update them. My idea was to have a unified location for such defaults. Again though, let's keep this separate from this PR.
Also, I think transparent black is a good choice.
@@ -27,15 +27,17 @@ canvas_definition: "canvas" _INLINE_WHITESPACE file_name _INLINE_WHITESPACE size | |||
item: rect | circle | ellipse | line | polygon | text | image | custom | |||
|
|||
color: identifier | hex_color | |||
clip: CLIP_TYPE | |||
CLIP_TYPE: "in" | "out" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also put these hardcoded values into the defaults.py file? For example, the file will contain placeholders. Then, we just have a function that loops over and replaces with the right values. Another option would be to read these keywords from this file into default.py. This is a big task but I think it could be really cool and really useful! Although, definitely for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind clarifying what you mean for this one @Daniel-Genkin? These are from the LARK grammar and are just part of the Syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand this. I meant that, if we decide to change the keywords for example, we can define them in one place, This is linked to the other comment.
Changes In This Clippy Update
Proper Clipping
Fixed clipping for:
Performed basic tests for these, but did not try any crazy edge cases. You can make a clipped object by doing this:
[New feature] Inner and Outer Clipping
I felt there was a need for two types of clipping when clipping an outlined object. This is due to the fact that you can either clip inside the stroke or outside the stroke and obtain a completely different effect. I explain the two types below with examples.
Inner Clipping
One can "inner clip", or clip inside the stroke by writing code like the following:
Yielding something like:
Inner Clipping
One can "outer clip", or clip outside the stroke by writing code like the following:
Yielding something more like this:
An Important Note
Both inner and outer clipping work perfectly for circles, rectangles, and ellipses. But for custom polygons, clipping can be quite difficult, so as of now, only inner clipping is supported. I hope to work on this again in the future to add outer clipping for custom polygons.