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

Allow passing in an ASSInspector to getLineBounds #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tgoyne
Copy link

@tgoyne tgoyne commented Dec 24, 2014

Reusing the ASSInspector instance speeds up ASSWipe from 15 seconds to 3 seconds on an arbitrary test file.

Reusing the ASSInspector instance speeds up ASSWipe from 15 seconds to 3
seconds on an arbitrary test file.
@line0
Copy link
Contributor

line0 commented Dec 28, 2014

implemented in a transparent way in @ba9664b4e18800d28f5d6f346d17436478e67ee7. thanks again for pointing it out. I'm not sure if there are any valid use cases to reuse an inspector instance across LineCollections - if you can think of one, feel free to reopen the issue and I'll add a parameter to pass one.

@torque
Copy link
Contributor

torque commented Dec 28, 2014

I haven't looked at how you're using LineCollections in ASSFoundation, so I'm just going to explain what ASSInspector does and let you draw your own conclusions.

A single ASSInspector instance should be guaranteed to be valid across the entire execution of a single automation script, if that script does not create any new styles. In fact, a single ASSInspector instance should be valid for a given file open in Aegisub at any time, as long as the headers and styles are not changed. Previously, temporarily installing/uninstalling fonts could cause problems, but should no longer be with registry-capable fontconfig.

The ASSInspector C library doesn't do much in the way of caching, and doesn't really matter anyway because this uses Inspector.moon, so let's walk through a run of Inspector.moon.

When an inspector is instantiated with Inspector(sub), the constructor does two main things, the first of which is to initialize the C library. Inspector.moon caches the C library instance it has initialized such that it will not be re-initialized unless:

  • The Inspector.moon script is reloaded by Automation (either with reload all, or by quitting Aegisub)
  • The Inspector:forceLibraryReload method is called on an initialized inspector object.

What this means is there is no point to initializing multiple Inspector objects in a single script. Because of the way Lua tables work, all initialized inspector objects should always point to the same initialized C library, even if forceLibraryReload is called on one of them.

The second thing the Inspector constructor does is to reread the script header. The collected header information is stored in the initialized inspector, so if multiple inspectors are initialized without the header having changed, each one will needlessly duplicate this information.

The intended (and completely undocumented) way for Inspector.moon to be used is for a script to initialize exactly one Inspector object, and call updateHeader(sub) on it when a header value has been changed.

As far as I have reasoned it out, this should work well. However, keeping the inspector's cached header up-to-date with what is actually in the script is deferred to the client script. As long as the client keeps track of when the header needs to be updated and updates it properly, a single Inspector instance should be valid at any time. The header should be updated if:

  • Any of the following header values are changed:
    • PlayResX
    • PlayResY
    • WrapStyle
    • ScriptType
    • ScaledBorderAndShadow
  • Any new styles are added.
  • Any existing styles are modified or deleted.

There previously was a bit of a kerfuffle with fonts, but since starting to use fontconfig with registry support this should no longer be a major concern on Windows. There's not really a good way to fix that particular issue on other operating systems, though, as far as I know.

Hopefully I didn't forget anything important.

@line0
Copy link
Contributor

line0 commented Jan 4, 2015

Thanks @torque for detailed description of how ASSInspector works. As far as my current solution is concerned, i don't think there's a lot of potential of stuff breaking, but i can see how it's still potentially inefficient (though much less so than before). I'll reopen this issue and get back to it later after having taken care of more urgent stuff.

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

Successfully merging this pull request may close these issues.

3 participants