ruffle-rs/rufflePublic

NotificationsYou must be signed in to change notification settings
Fork967
Star17.7k

Improve TextField.autoSize, implement TextField.background#3163

Merged

Herschel merged 3 commits intoruffle-rs:masterfrom

adrian17:autosize-improvements

Feb 13, 2021

Merged

Improve TextField.autoSize, implement TextField.background#3163
Herschel merged 3 commits intoruffle-rs:masterfrom
adrian17:autosize-improvements

Conversation

Copy link

Collaborator

adrian17 commentedFeb 8, 2021•
edited
Loading

First commit makes TextField better respect properties of autoSize: with "left", it extends to the right from_x; with "right", it extends to the left from_x+_width. This doesn't affect text layout; just where the text field appears in the end.
This is not a full fix of any issue; in fact, this doesn't fix completely any issues, just improve them a bit. In particular, various missing parts I noticed:

there are minor margin-related differences from Flash (you can see the last digit "0" being cut off on screenshots below; that's also why tests run in approximate mode)
text re-layout should occur lazily (think likeconsole.log(el.offsetLeft) triggers reflow in JS), not on each property set
autoSize="right" should overruletextFormat.align and align text to right;
InText shown in SWF is not in the right place and cut off, and text shown when hovering is inaccurate too #3156, it still sometimes behaves weirdly and I don't fully understand why.

As for improvements:

Hovering text is broken in Sonny #1095 - tooltips no longer appears on top of cursor, so buttons are finally clickable (the layout is still as broken as before though)
Nanaca Crash: Misaligned 'record' numbers #1440 - "new record" etc appear where it should, but slightly overlap with the "m"
Text shown in SWF is not in the right place and cut off, and text shown when hovering is inaccurate too #3156 - text appears closer to where it should be, but still not 100% correct.
makes borders refresh on re-layout ifautoSize is enabled
implementsTextField.background flag - before, we would draw background wheneverborder=true, which wasn't correct.

Screenshots:

test, Flash Player (respectively: reference text, dynamic text with no autoSize, then texts with autoSize=left, center, right):
test, Ruffle, before PR:
test, Ruffle, with PR:
Sonny:
Text shown in SWF is not in the right place and cut off, and text shown when hovering is inaccurate too #3156 no hover:
Text shown in SWF is not in the right place and cut off, and text shown when hovering is inaccurate too #3156 on hover (better than before but sometimes still weirdly too much too the right):

Nanaca Crash: Misaligned 'record' numbers #1440:

adrian17 force-pushed theautosize-improvements branch from9b18211 to5cc767eCompare

February 8, 2021 22:08

seanpm2001 reviewed

Feb 11, 2021

View reviewed changes

Copy link

seanpm2001 left a comment•
edited
Loading

Choose a reason for hiding this comment

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

It looks good. Everything is clean and functional.

Copy link

Member

kmeisthax commentedFeb 11, 2021•
edited
Loading

Same here.

I'll point out that you should also make sure the otherEditText tests don't have a rendering regression. I structured them to double as both traceable and renderable regression tests, so you should run them on Desktop & compare with Flash Player orruffle/master as well.

Copy link

Contributor

danielhjacobs commentedFeb 11, 2021•
edited
Loading

As the author of#3156 this looks promising, though obviously it would be better for me if it fully fixed that issue.

Semi-related; this may not be the best place for this comment, but I thought I'd mention this since my issue was mentioned:

I added more SWFs with the same problem to the issue. They are nearly identical files, but maybe they will give a clue as to the behavior that you "don't fully understand". I thought they were redundant, but better to include too much detail than too little.

Edit: Further testing revealed a slight difference in the problems with some of the SWFs versus others. See latest comment on the issue for further details since this is not the place for me to mention them.

Copy link

CollaboratorAuthor

adrian17 commentedFeb 11, 2021

I'll point out that you should also make sure the other EditText tests don't have a rendering regression

I hoped the existing tests would "handle themselves" without having me actually look at them (especially if they pass) :)
But sure, I can take a look at them (I don't think most of them actually useautoSize, so hopefully they shouldn't be affected).

Copy link

CollaboratorAuthor

adrian17 commentedFeb 11, 2021

@craniumcadoo FYI:

Upon moving the mouse to hover over (4), then (5), then back to (4), then ... repeatedly, the text just keeps moving further right, until it is eventually offscreen completely.

From what I see, my patch seems to fix this particular behavior.