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

renderExtras is inside the chart contentRect, so renderExtras should be clipped #208

Closed
wants to merge 1 commit into from

Conversation

liuxuan30
Copy link
Member

When I tried to add something more in renderExtras, it will draw outside of the chart contentRect. I think renderExtras should also be clipped. How do you think?

…also be clipped, and should be done before we restore the context
@liuxuan30
Copy link
Member Author

@danielgindi today I kind of know why you don't clip drawExtras, because you are drawing dots in the line chart view, and you don't want the dot to be clipped while the dot is at the edge.

However, when I tried to add some additonal stuff on the chart, I make use of drawExtras and see it draws beyond the rect, so I think it is reasonable to be clipped.

Maybe we should reconsider the structure here, as sometimes people want drawExtras to be clipped while sometimes not

@danielgindi
Copy link
Collaborator

Actually you should make sure you don't draw something that is out of bounds...
You could clip yourself when you override drawExtras, but the best practice is to just not draw anything that shouldn't be visible

@liuxuan30
Copy link
Member Author

@danielgindi but you draw the line chart dots in drawExtras, and it is drawing out of bounds :-)
though it will not exceeds the axis

@liuxuan30 liuxuan30 closed this Jul 20, 2015
@danielgindi
Copy link
Collaborator

Yes, specifically those dots look bad when clipped, so we are drawing them even if they are bleeding a little bit. But eventually when you subclass it is up to you if you clip or not :-)

Take into consideration that the fact that you are clipping, in itself it requires more resources for drawing. It comes with a cost.

@liuxuan30 liuxuan30 deleted the renderExtraBug branch July 20, 2015 09:13
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.

2 participants