Define the PathProvider interface to create Shape objects for deferred construction of Path objects. #404
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shlzxjp
requested review from
kevingpqi123,
domchen,
Hparty and
lvpengwei
as code owners
domchen
reviewed
Dec 26, 2024
include/tgfx/core/PathProvider.h
Outdated
| * Note: This method may be called multiple times. If constructing the Path object is | ||
| * time-consuming, consider caching the Path object after it is created. | ||
| */ | ||
| virtual Path getPath() = 0; |
There was a problem hiding this comment.
这个方法需要标记const，确保调用的时候没有写入操作才能保证线程安全。
There was a problem hiding this comment.
这个方法也要增加一个resolutionScale精度参数，传递下去，以防外部生成Path的过程是需要使用Stroke::applyToPath(Path* path, float resolutionScale = 1.0f)的。
domchen
reviewed
Dec 26, 2024
src/core/shapes/ExternalShape.cpp
Outdated
|
|
||
| Path ExternalShape::getPath(float resolutionScale) const { | ||
| auto finalPath = getPathInternal(); | ||
| finalPath.transform(Matrix::MakeScale(resolutionScale)); |
There was a problem hiding this comment.
这个参数的含义理解错了，看一下注释。它不改变Path的大小。这是个精度缩放值。在Stroke相关的操作里才会用到。
domchen
reviewed
Dec 26, 2024
src/core/shapes/ExternalShape.cpp
Outdated
Comment on lines
75
to
79
| if (path.isEmpty()) { | ||
| const auto tmpPath = const_cast<Path*>(&path); | ||
| *tmpPath = provider->getPath(); | ||
| } | ||
| return path; |
There was a problem hiding this comment.
标记了const的方法又进行数据写入，会导致线程安全问题。我们的Shape::getPath()方法已经加过注释了，每次调用都会生成一个新的Path，并不缓存。去掉这块的特殊逻辑。
domchen
reviewed
Dec 26, 2024
src/core/shapes/ExternalShape.cpp
Outdated
| } | ||
|
|
||
| UniqueKey ExternalShape::getUniqueKey() const { | ||
| return PathRef::GetUniqueKey(getPathInternal()); |
There was a problem hiding this comment.
这里直接像EffectShape一样，定义一个自身独立的UniqueKey，不能从Path上获取，整个Shape对象都是immutable的才能保证线程安全，Path不能缓存
domchen
reviewed
Dec 26, 2024
src/core/shapes/ExternalShape.cpp
Outdated
Comment on lines
59
to
61
| auto finalPath = getPathInternal(); | ||
| finalPath.transform(Matrix::MakeScale(resolutionScale)); | ||
| return finalPath.getBounds(); |
There was a problem hiding this comment.
应该给PathProvider定义一个getBounds方法。避免要生成Path才能计算bounds，这就违背了延迟计算的初衷了。getBounds是没有渲染的时候就要调用的。
domchen
reviewed
Dec 26, 2024
src/core/shapes/ExternalShape.h
Outdated
| #pragma once | ||
|
|
||
| namespace tgfx { | ||
| class ExternalShape : public Shape { |
domchen
reviewed
Dec 26, 2024
include/tgfx/core/PathProvider.h
Outdated
Comment on lines
34
to
35
| * Note: This method may be called multiple times. If constructing the Path object is | ||
| * time-consuming, consider caching the Path object after it is created. |
Update comment
Remove default parameter values for getPath and getBounds methods.
domchen
approved these changes
Dec 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
定义PathProvider接口，通过PathProvider创建Shape，用于延迟构建Path对象。