PyonFX icon indicating copy to clipboard operation
PyonFX copied to clipboard

Shape: add exact bounding box calculation and add align method

Open Funami580 opened this issue 3 years ago • 1 comments

I hope I got it all right. What do you think?

Funami580 avatar Feb 28 '22 00:02 Funami580

Hey there! Thank you for the PR, much appreciated.

I will need to look into details of the logic behind your implementation (I see that you also linked stackoverflow). But for now, here's some thoughts: what about not having a separated method, but just move bounding_exact into bounding and introduce a new parameter like exact? Moreover, we will need to update docstring by clarifying that the exact method is more expensive than the standard one, so it should be used only when needed.

And finally, it is pretty confusing to introduce the "an" parameter in the move method. E.g. what does it mean to move a shape from position (0,0) to position (100,20) with an=3 or an=6? Alignment for subtitles generally affects both starting position and final position, so it is really strange... Indeed, you only use alignment when you want to "align to center" your shape... At this point, I think it is better to move the logic you've implemented to a separated method, like "Shape.center".

CoffeeStraw avatar Feb 28 '22 00:02 CoffeeStraw