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

refactor: avoid copy all attributes to parsed style #1807

Open
wants to merge 2 commits into
base: release
Choose a base branch
from

Conversation

Aarebecca
Copy link
Contributor

@Aarebecca Aarebecca commented Oct 28, 2024

G/packages/g-lite/src/css/StyleValueRegistry.ts 中将 attribuets 中属性拷贝至 parsedStyle 会花费一定时间,并造成内存开销增加。这个 PR 对相关逻辑进行改造,只有真正解析转换的属性才会存放至 parsedStyle,其余地方需根据情况访问。
尽管导致了维护成本增加,但一定程度上能够优化性能

image

@Aarebecca Aarebecca force-pushed the refactor/parsed-style branch 2 times, most recently from b356280 to 5d202cf Compare October 28, 2024 07:18
@wang1212
Copy link
Member

wang1212 commented Oct 28, 2024

如果这样改的话,估计一定程度上会导致程序流程不清晰,比如:

  • process 过程应该处理哪些 attributes 到 parsedStyle
  • 渲染过程中哪些属性应该从 attributes 取,哪些要从 parsedStyle 取,明确的标准有没有

另外,现在对象实例上存储属性的字段应该有 config/attributes/parsedStyle 等等,得了解下相关的字段代表的含义,为什么要这么设计,避免引起其它副作用

@Aarebecca
Copy link
Contributor Author

Aarebecca commented Oct 28, 2024

如果这样改的话,估计一定程度上会导致程序流程不清晰,比如:

  • process 过程应该处理哪些 attributes 到 parsedStyle
  • 渲染过程中哪些属性应该从 attributes 取,哪些要从 parsedStyle 取,明确的标准有没有

另外,现在对象实例上存储属性的字段应该有 config/attributes/parsedStyle 等等,得了解下相关的字段代表的含义,为什么要这么设计,避免引起其它副作用

是的,这是这个 PR 导致的一些问题。一个简单的判定标准就是 packages/g-lite/src/css/StyleValueRegistry.ts 中processProperties 方法里解析的下列属性
image

property property property
fill stroke shadowColor
filter radius lineDash
points d text
clipPath transform transformOrigin
markerStart markerMid markerEnd
isClosed miterLimit -

对于 G 层面来说可能需要 G 的开发者明确知晓各个属性该从 attributes 还是 parsedStyle 中取,对于上层框架开发者,可以使用导出的工具方法 getParsedStyle 取值:

import { Rect, getParsedStyle } from '@antv/g';

const rect = new Rect({
  style: {
    width: 100,
    height: 100,
    radius: 10,
  },
});

const radius = getParsedStyle(rect, 'radius');
// -> [10, 10, 10, 10]

@Aarebecca
Copy link
Contributor Author

@wang1212 对于上层框架来说,有 10% 的时间用在拷贝属性到上来说是一个不小的开销。
也是过用 Proxy 的方式,先从 parsedStyle 中取值,找不到的话再到 attributes 中取,不过这种方式性能甚至进一步降低

@wang1212
Copy link
Member

如果这样改的话,估计一定程度上会导致程序流程不清晰,比如:

  • process 过程应该处理哪些 attributes 到 parsedStyle
  • 渲染过程中哪些属性应该从 attributes 取,哪些要从 parsedStyle 取,明确的标准有没有

另外,现在对象实例上存储属性的字段应该有 config/attributes/parsedStyle 等等,得了解下相关的字段代表的含义,为什么要这么设计,避免引起其它副作用

是的,这是这个 PR 导致的一些问题。一个简单的判定标准就是 packages/g-lite/src/css/StyleValueRegistry.ts 中processProperties 方法里解析的下列属性 image

property property property
fill stroke shadowColor
filter radius lineDash
points d text
clipPath transform transformOrigin
markerStart markerMid markerEnd
isClosed miterLimit -
对于 G 层面来说可能需要 G 的开发者明确知晓各个属性该从 attributes 还是 parsedStyle 中取,对于上层框架开发者,可以使用导出的工具方法 getParsedStyle 取值:

import { Rect, getParsedStyle } from '@antv/g';

const rect = new Rect({
  style: {
    width: 100,
    height: 100,
    radius: 10,
  },
});

const radius = getParsedStyle(rect, 'radius');
// -> [10, 10, 10, 10]

看起来还会影响用户侧,这个可能会导致用户产生困惑,看看能不能搞清楚为啥要区分 attributes 和 parsedStyle,是为了兼容 dom 标准还是为了啥,能不能直接从架构设计层面精简掉

@wang1212
Copy link
Member

@wang1212 对于上层框架来说,有 10% 的时间用在拷贝属性到上来说是一个不小的开销。 也是过用 Proxy 的方式,先从 parsedStyle 中取值,找不到的话再到 attributes 中取,不过这种方式性能甚至进一步降低

开销确实比较大,其实问题在于 attributes 和 parsedStyle 是不是算是内部 object 对象的实现细节,能不能通过 getter 和 setter 把内部实现隐藏掉,用户侧和渲染层不需要感知某个子模块的实现细节

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