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

font followups #167

Open
1 of 3 tasks
Thorin-Oakenpants opened this issue May 20, 2022 · 5 comments
Open
1 of 3 tasks

font followups #167

Thorin-Oakenpants opened this issue May 20, 2022 · 5 comments
Labels

Comments

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented May 20, 2022

ToDo

Ideas and things to follow up on from perf work

  • add a transform scale (width + height), e.g. transform: scale(1.0361118, 0.91866667);
    • I assume this then really needs ToNumber to fully leverage ?
  • add ToNumber methods (see below original post)
  • make sure style is being reset for each test
    • that's this bit, not the font family
    • I remember reading about it in one of our FPing papers that this was important, it was an improvement discovered by researchers when doing tests in the wild for stability, and fpjs updated to do the same when told. But I can't find it. FYI, fpjs puts each test in it's own span (createSpan....), whereas your test re-uses and only sets it once
    • I need to find it again, because it's bugging me: and then we evaluate what its all about and if we need to do anything (we need to know what causes the issues in that research paper's results)
    • I do note that the style is full of !important;

reference

	const pixelsToInt = pixels => Math.round(+pixels.replace('px',''))
	const originPixelsToInt = pixels => Math.round(2*pixels.replace('px', ''))

       // and

	const pixelsToNumber = pixels => +pixels.replace('px','')
	const originPixelsToNumber = pixels => 2*pixels.replace('px', '')

OP from other post

what do you mean tampering noise

Ahh, so I tired it, and I see what you mean: I didn't check the differences in measurements. So it splits the results (for me) into three - those that don't use pixelsToNumber, those that use originPixelsToNumber and those that don't use either

Since I'm expecting them to all be the same, and I need 4/7 to be the same to determine a result, I end up flummoxed and return a red lie. This is my lie detector - comparing sets. I think yours would have the same issue

noise


But ... I could still use this. Add more methods (the perf cost is very cheap it seems) and Sets, use the ToInt ones for lies, but display and show and record the ToNumber

And get transform:scale going

I'm going to open a new issue on these things and keep this one for perf

Originally posted by @Thorin-Oakenpants in https://github.com/arkenfox/TZP/issues/34#issuecomment-1132631154

@Thorin-Oakenpants
Copy link
Contributor Author

Ahh, so I tired it,

Tried it. This seems like a good point to go have a 16 hr sleep

@Thorin-Oakenpants
Copy link
Contributor Author

  • make sure style is being reset for each test

If I understand correctly the relevant code is here and and getDimensions reapplies the style when measuring

TZP/js/fonts.js

Lines 399 to 400 in 36f9d92

const style = getComputedStyle(span)
const dimensions = getDimensions(span, style)

style is a const and therefore can't change (from what we set)

and here is the hardcoded style we set

TZP/js/fonts.js

Lines 285 to 289 in 36f9d92

doc.getElementById(id).innerHTML = `
<style>
#${id}-detector {
--font: '';
position: absolute !important;

is that correct @abrahamjuliot

--

but const style is in two places

TZP/js/fonts.js

Line 335 in 36f9d92

const style = getComputedStyle(span)

once when we create the elements, and again on every looped font. Why can't the font loop just use the global const? - another @abrahamjuliot question

@abrahamjuliot
Copy link
Collaborator

style is a const...

This just prevents resetting the obj like style = true, but since it is an object, the key/values can be set or re-computed.

Object.freeze(obj) can be used to prevent re-setting object values, but in our case, we are getting the style object from the dom which is re-computed when the element and styles are rendered to the page. So, each time the styles or elements change, we need to call getComputedStyle to get the newly rendered dimensions.

@abrahamjuliot
Copy link
Collaborator

each time the styles or elements change, we need to call getComputedStyle

Correction. This is only necessary if we are changing the element or stylesheet, so I think we don't need to call getComputedStyle each time. The values within our const style object should re-compute.

@Thorin-Oakenpants
Copy link
Contributor Author

this line didn't affect perf at all (because nothing changed?)

TZP/js/fonts.js

Line 391 in 9b4a517

const style = getComputedStyle(span)

but it insures(?) the style hasn't changed - this is the bit I need to find in the FPing paper - it was like 3 years ago I modified arthur's fallback poc after reading about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants