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

isPartiallyOnScreen method #4856

Merged
merged 11 commits into from
Apr 28, 2018
Merged

isPartiallyOnScreen method #4856

merged 11 commits into from
Apr 28, 2018

Conversation

keyurpatel
Copy link
Member

@keyurpatel keyurpatel commented Mar 26, 2018

Close #4823,

Issue: to know if the object is partially off screen, completely off screen.

Solution: create a simple helper methods which return boolean for object partially off screen isPartiallyOnScreen, completely offscreen isOffScreen.

isPartiallyOnScreen
isOffScreen
/**
* Checks if object is partially contained within the canvas with current viewportTransform
* the check is done stopping at first point that appears off screen
* @param {Boolean} [calculate] use coordinates of current position instead of .oCoords
Copy link
Member

@asturur asturur Mar 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use coordinates of current position, calculated just in time, instead of precalculated .aCoords
Also the check is made with intersection, the old comment does not really apply anymore.
the check is done stopping at first point that appears off screen is not really relevant in this implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply to the other function too.

cObj.top = -1000;
cObj.setCoords();
assert.equal(cObj.isPartiallyOnScreen(cObj.aCoords), false, 'object is not partially on screen because it is outside the canvas');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have more test to cover and edge cases i believe. Intersect can return true for coincidents segments too.

@asturur asturur merged commit fcef0a3 into fabricjs:master Apr 28, 2018
@asturur asturur mentioned this pull request May 8, 2018
thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
* appears

* new methods

isPartiallyOnScreen
isOffScreen

* UT

* test

* comment improved

* removed isOffScreen

* removed isOffScreen

* Update object_geometry.mixin.js

* Update object_geometry.mixin.js

* lint...

* omg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants