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

Add an interface to the Path to support constructing Path using SVG #284

Merged
merged 11 commits into from
Oct 30, 2024

Conversation

YGaurora
Copy link
Collaborator

Support SVG-style arc descriptions.
Get the last point of the Path.

include/tgfx/core/Path.h Outdated Show resolved Hide resolved
/**
* Returns last point on Path in lastPt. Returns false if point array is empty.
*/
bool getLastPt(Point* lastPt) const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

避免使用缩写,getlastPoint(),参数lastPt也对应修改一下。

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个接口是外部要调用还是只是我们内部实现arcTo函数需要?如果只是内部需要,我们直接访问PathKit的接口就行,不用暴露这个公开接口。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

只是内部需要,但是我认为这个接口还是对用户来说有价值的,比如现有的addArc其实不需要输入结束点,在add后需要知道终点是哪儿还需要自己计算。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

并且是为了svg模块去使用,也算是外部的调用

include/tgfx/core/Path.h Outdated Show resolved Hide resolved
include/tgfx/core/Path.h Outdated Show resolved Hide resolved
include/tgfx/core/Path.h Outdated Show resolved Hide resolved
include/tgfx/core/Path.h Outdated Show resolved Hide resolved
src/core/Path.cpp Outdated Show resolved Hide resolved
* Point and (x, y) if both are greater than zero but too small.
* arcTo() appends up to four conic curves.
* arcTo() implements the functionality of SVG arc, although SVG sweep-flag value is opposite the
* integer value of sweep; SVG sweep-flag uses 1 for clockwise,while counter-clockwise direction
Copy link
Collaborator

Choose a reason for hiding this comment

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

参数里已经没有sweep flag了

Copy link
Collaborator Author

@YGaurora YGaurora Oct 29, 2024

Choose a reason for hiding this comment

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

这里应该是指的svg里面的参数
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

用Github Copilot重写一下这段注释吧,prompt:Rewrite it to sound more natural for a native speaker.

* @param xAxisRotate x-axis rotation in degrees; positive values are clockwise
* @param largeArc chooses smaller or larger arc
* @param reversed Choose the rotation clockwise direction.(clockwise = false)
* @param endPt end of arc
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里endPt和参数命名不一致

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改

/**
* Specify whether the arc is greater than 180 degrees pair or less than 180 degrees pair.
*/
enum class ArcSize : uint8_t {
Copy link
Collaborator

Choose a reason for hiding this comment

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

命名加上Path前缀: PathArcSize

Comment on lines 245 to 246
Matrix pointTransform;
pointTransform.setRotate(-xAxisRotate);
Copy link
Collaborator

Choose a reason for hiding this comment

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

可以合并为 auto pointTransform = Matrix::MakeRotate()

Matrix pointTransform;
pointTransform.setRotate(-xAxisRotate);

Point transformedMidPoint;
Copy link
Collaborator

Choose a reason for hiding this comment

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

声明变量都记得赋初始值 Point::Zero(), 不然很多隐藏bug。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改

bool Path::getLastPoint(Point* lastPoint) const {
SkPoint skPoint;
if (pathRef->path.getLastPt(&skPoint)) {
lastPoint->set(skPoint.fX, skPoint.fY);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里对lastPoint的指针访问是否需要判空?检查一下PathKit里的对应实现是否判空,如果那边判空了,这里也要加上。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

需要判空,已补充

Comment on lines 106 to 110
Small_ArcSize,
/**
* larger of arc pair
*/
Large_ArcSize,
Copy link
Collaborator

Choose a reason for hiding this comment

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

把_ArcSize后缀去掉,Skia里这么命名是因为没有使用enum class,怕重名。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已修改

Comment on lines 245 to 246
Matrix pointTransform = Matrix::MakeRotate(-xAxisRotate);
Point transformedMidPoint = Point::Zero();
Copy link
Collaborator

Choose a reason for hiding this comment

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

类型声明推荐尽可能多使用auto,让编译器推理,可读性也更好。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

涉及的代码都已修改

@@ -415,4 +542,15 @@ int Path::countPoints() const {
int Path::countVerbs() const {
return pathRef->path.countVerbs();
}

bool Path::getLastPoint(Point* lastPoint) const {
if (!lastPoint) return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

避免写这种单行if的代码,严格都加上大括号,不然if条件去掉容易出别的bug。代码是写给人类阅读的,不用节省行数。

@domchen domchen merged commit b4a1231 into main Oct 30, 2024
8 checks passed
@domchen domchen deleted the feature/yg_add_interface_to_path branch October 30, 2024 02:42
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