ASTextNode2 to ignore certain text alignments while calculating intrinsic size #1166
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
nguyenhuy
changed the title
nguyenhuy
changed the title
…sic size ASTextNode2 uses ASTextLayout to calculate its layout and bounding rect. When the constrained width that is used for layout calculation is inf/max, ASTextLayout doesn't ignore right/center/natural text alignment but takes it into account. That results in an unreasonable size (and position). Fix by detecting when the node is calculating intrinsic size and force its layout alignment to be left. Other alignments should still work when the max width is finite/reasonable.
nguyenhuy
force-pushed
the
HN-ASTextNode2-Alignment-Inf-Width
branch
from
f995daf to
a139d66
Compare
Adlai-Holler
approved these changes
Oct 10, 2018
There was a problem hiding this comment.
Hey Huy! This fix seems fine for the problem, though I would love to get @wiseoldduck 's take as well since he knows the text situation super good. I pestered him over chat.
wiseoldduck
approved these changes
Oct 11, 2018
Source/ASTextNode2.mm
Outdated
|
|
||
|
|
||
| // If constrained width is max/inf, the text node is calculating its intrinsic size. | ||
| const BOOL calculatingIntrinsicSize = (_textContainer.size.width >= ASTextContainerMaxSize.width); |
There was a problem hiding this comment.
Does Texture ever use the isVerticalForm property? We should probably just delete it, but if not maybe check for it here.
Source/ASTextNode2.mm
Outdated
| @@ -321,18 +324,31 @@ - (NSArray *)exclusionPaths | |||
| return _textContainer.exclusionPaths; | |||
| } | |||
|
|
|||
| - (void)prepareAttributedString:(NSMutableAttributedString *)attributedString | |||
| - (void)prepareAttributedString:(NSMutableAttributedString *)attributedString forIntrinsicSize:(BOOL)isForIntrinsicSize | |||
There was a problem hiding this comment.
This method name implies the second parameter is a CGSize, to me something like isForIntrinsicSize would imply a BOOL.
nguyenhuy
force-pushed
the
HN-ASTextNode2-Alignment-Inf-Width
branch
from
cf8991a to
24365db
Compare
|
Awesome! Thank you for reviewing, @Adlai-Holler and @wiseoldduck. |
|
🚫 CI failed with log |
|
🚫 CI failed with log |
nguyenhuy
added a commit
that referenced
this pull request
Oct 11, 2018
…nsic size (#1166) ASTextNode2 uses ASTextLayout to calculate its layout and bounding rect. When the constrained width that is used for layout calculation is inf/max (e.g when the node is inside a horizontal stack), ASTextLayout doesn't ignore its right/center/natural text alignment but takes it into account. That results in an unreasonable size (and position). Fix by detecting when the node is calculating intrinsic size and force its layout alignment to be left. Other alignments should still work when the max width is finite/reasonable.
mikezucc
pushed a commit
to mikezucc/Texture
that referenced
this pull request
Nov 7, 2018
…nsic size (TextureGroup#1166) ASTextNode2 uses ASTextLayout to calculate its layout and bounding rect. When the constrained width that is used for layout calculation is inf/max (e.g when the node is inside a horizontal stack), ASTextLayout doesn't ignore its right/center/natural text alignment but takes it into account. That results in an unreasonable size (and position). Fix by detecting when the node is calculating intrinsic size and force its layout alignment to be left. Other alignments should still work when the max width is finite/reasonable.
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.
ASTextNode2 uses ASTextLayout to calculate its layout and bounding rect. When the constrained width that is used for layout calculation is inf/max (e.g when the node is inside a horizontal stack), ASTextLayout doesn't ignore its right/center/natural text alignment but takes it into account. That results in an unreasonable size (and position).
Fix by detecting when the node is calculating intrinsic size and force its layout alignment to be left. Other alignments should still work when the max width is finite/reasonable.