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

[ASDisplayNode] Pass drawParameter in rendering context callbacks #248

Merged
merged 3 commits into from
May 8, 2017

Conversation

maicki
Copy link
Contributor

@maicki maicki commented May 6, 2017

The reasoning for this change is that developers came up with all kinds of interesting ideas how to use properties in the ASDisplayNodeContextModifier blocks as it's running on non main properties like backgroundColor is not accessible. It is possible to access this properties on the main thread though and as we know drawParametersForAsyncLayer is called on main, developers can store this kind of properties within the draw parameters and use this in the ASDisplayNodeContextModifier blocks. To be able to do that we actually have to pass them in what this PR is planning to do.

This is basically an API breakage change and I'm happy to change if we would like to deprecate the old API and move over to this one, but as willDisplayNodeContentWithRenderingContext and didDisplayNodeContentWithRenderingContext is in the beta header I don't think we have to worry too much, furthermore it's not too hard to change to the new ASDisplayNodeContextModifier block as it's just adding a parameter.

@ghost
Copy link

ghost commented May 6, 2017

🚫 CI failed with log

@maicki maicki force-pushed the MSPassDrawParameterInRenderingContext branch from a4e80d9 to de8967b Compare May 6, 2017 15:57
@ghost
Copy link

ghost commented May 6, 2017

🚫 CI failed with log

@ghost
Copy link

ghost commented May 6, 2017

🚫 CI failed with log

@ghost
Copy link

ghost commented May 6, 2017

1 Warning
⚠️ Please ensure license is correct for LayoutExampleNodes.m:

//
//  LayoutExampleNodes.m
//  Texture
//
//  Copyright (c) 2014-present, Facebook, Inc.  All rights reserved.
//  This source code is licensed under the BSD-style license found in the
//  LICENSE file in the /ASDK-Licenses directory of this source tree. An additional
//  grant of patent rights can be found in the PATENTS file in the same directory.
//
//  Modifications to this file made after 4/13/2017 are: Copyright (c) 2017-present,
//  Pinterest, Inc.  Licensed under the Apache License, Version 2.0 (the "License");
//  you may not use this file except in compliance with the License.
//  You may obtain a copy of the License at
//
//      http://www.apache.org/licenses/LICENSE-2.0
//

    

Generated by 🚫 Danger

Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Perfect! I agree let's not worry about the API change. Getting more immutable state captured into these draw parameters, and propagated to the asynchronous draw code safely is the name of the game. Land away!

@maicki maicki merged commit 8692428 into master May 8, 2017
@maicki maicki deleted the MSPassDrawParameterInRenderingContext branch May 9, 2017 18:20
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