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

Feature/mac compatibility #31

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

taxilian
Copy link
Contributor

@taxilian taxilian commented Nov 7, 2021

Not a lot of major things, but xdg-open doesn't work on macOS so I added detection for the platform to fix that and make those functions work. Also found a place where a platform-specific separator broke functionality on linux and mac and one other spot which honestly shouldn't need a check, but somehow it did. I was too lazy to track down exactly why since this easy fix fixes it =]

@DCStrato
Copy link
Collaborator

DCStrato commented Nov 8, 2021

Awesome. I had not looked at Mac or Linux issues. I don't own any recent Mac hardware to test with. Service went swimmingly Yesterday. Still that one memory leak on my hardware, not sure if you are seeing it or if it is isolated to something else I am using. Maybe for in a future release. The tech that typically does service setups is still asking for "verse" control and has also requested source loads offer a way to supply lyrics directly for responsive reading and maybe even optionally point to different text objects to modify. I am still trying to come with how to keep up with verse selections per Prepared Lyric in a list, given OBS does not offer a way to easily edit tables in the UI. I will probably implement it in the source load first just to confirm it is working. It seems most people are generating new UI objects inside a subgroup for each item in a parent list (a list per list item concept). It would start to look a bit like modifying HotKeys in OBS setup. Eight prepared songs would have a subgroup open with eight requests for verse filters in a CSV list somewhat like meta tags. Another option is more file intensive and could read the current verse limits saved with each song with an option to use them as-is or modify them (list per file). Neither are really pretty answers. But then OBS is rarely "pretty" when it comes to UI. Thanks again for your help on Mac and Linux issues!!

@taxilian
Copy link
Contributor Author

taxilian commented Nov 8, 2021

Worth noting that the "fade" functionality does not work on mac; I have no idea if it does on linux or not. The text display elements work quite a bit differently on mac and don't have the same options. I'm not super concerned about it, but thought others might want to know.

@DCStrato
Copy link
Collaborator

DCStrato commented Nov 9, 2021 via email

@DCStrato
Copy link
Collaborator

DCStrato commented Nov 9, 2021

Taxillian,

The fade function relies on the capability of Windows GDI+ text objects that apparently are not supported in the Mac version. The Mac version currently uses the older FreeType 2 text objects that have a fixed Top and Bottom color gradient with alpha channels. It will be possible to support those to the extent that we currently are supporting gradient text in GDI+. The properties only keep a single Alpha channel value and that value has to be changed to cause the fade effect. So we have to try to remember what it was and put it back to that value during a Fade-In. Using only 0-100% alpha (opacity) values is easier as those are fixed ranges external to the actual alpha values. This new release of Lyrics offers greater GDI+ support for gradient fades. After it releases, I can try to add backward support for the older FreeType2 text formats fairly quickly in a future update. The code is pretty easy, but I do not want to risk having to go back and re-test this close to a major release, nor does it make sense to try and modify the current release at this point in time. The FreeType2 looks almost identical to the GDI+ gradient. The FreeText2 format is just RGBA with A being its opacity. So the functions getSourceOpacity() and apply_source_opacity() will need to be modified to support the older format. Fortunately, Amirchev's work in refactoring Lyrics to better isolate these functions will make this a fairly easy update. :)

D.C.

@amirchev
Copy link
Owner

amirchev commented Nov 10, 2021

@taxilian great to have you working on Mac compatibility, because I have really only tested this on Windows and am relying on user feedback for other systems. The only catch is we have a large refactor and update coming from the cleanup branch. So, we will manually add this code into the update after releasing 2.0. After that's done, I'll close this request. Looks great.

@taxilian
Copy link
Contributor Author

My time is sometimes a bit hit and miss -- I use this every Sunday, but in general it's kindof a "if it ain't broke, don't fix it" thing =] That said, feel free to ping me if you need something checked on mac and I'll try to take a look.

I'm far from a lua expert, but I have enough dev experience to generally figure out solutions when I need to.

the changes in this PR should be pretty easily applied however you want.

@taxilian
Copy link
Contributor Author

Also to be clear I am not personally too concerned about the fade -- I haven't seen it, so i'm not sure if I'd want to use it or not, but I'm fine as-is, I just thought it might be good to have that be a known thing =]

@jacksonj04
Copy link

Is there any interest in getting this merged? I've got a local branch with these changes reapplied over the latest main, happy to either open a new PR or upload for @taxilian to nab.

@amirchev
Copy link
Owner

amirchev commented Apr 8, 2023

Is this okay @taxilian ? This branch has conflicts

@taxilian
Copy link
Contributor Author

I'm no longer doing livestreams, so haven't used this in many months -- happy to help if there is anything you don't understand and want to import or you can use this or close it however you think best for the project. No need to worry about my feelings or giving credit, I just wanted to contribute back to something I was using if it was helpful

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.

4 participants