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

Rework bitmap controls #2 #317

Merged
merged 29 commits into from
Jul 2, 2023

Conversation

olilarkin
Copy link
Contributor

@sdatkinson here is a new PR, commenting out the color picker and adapting the about box a bit

I really think it's better not to squash merge PRs, but to include a merge-commit. I try to make a big effort to make each commit clean and atomic, so that individual changes can be reverted, without reverting an entire PR.

@olilarkin olilarkin force-pushed the rework-bitmap-controls branch from 6b44cb6 to 045b589 Compare July 1, 2023 08:56
Copy link
Owner

@sdatkinson sdatkinson left a comment

Choose a reason for hiding this comment

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

Well, this isn't fun.

But, as I was reviewing this, I've realized that Ronduit is not a free font, which puts me in a tough spot wrt the open-source MIT license that this repo has.

One way to solve this would be to remove Ronduit from the PR. But, if you have any other ideas, I'm happy to hear them. But I don't feel comfortable accepting the PR without some change, unfortunately.

@olilarkin
Copy link
Contributor Author

olilarkin commented Jul 2, 2023

I was also dismayed to find that the font was not free after the graphics had been made previously based on a flattened photoshop file. I got the source photoshop file in order to create higher res resources, and only then found out the name of the font. But the license seems ok for NAM if it's open source and free.

@olilarkin
Copy link
Contributor Author

olilarkin commented Jul 2, 2023

Better to include the license, obtain explicit permission from the author or change the font, IMO.

It should be possible to find something suitable with a permissive license:

https://fonts.google.com/?preview.text=NEURAL%20AMP%20MODELER&preview.size=32&preview.text_type=custom&category=Serif,Sans+Serif,Display

@sdatkinson
Copy link
Owner

It should be possible to find something suitable with a permissive license:

Thanks for the suggestion Oli! Michroma looks fantastic to me as something in a similar style. In fact, I might even prefer it over Ronduit 🙂

You can change the PR, or I can probably handle it if you'd like; just let me know 👍🏻

@olilarkin olilarkin force-pushed the rework-bitmap-controls branch from 045b589 to 279d04a Compare July 2, 2023 21:30
@olilarkin
Copy link
Contributor Author

updated. Not 100% if it works on windows

@sdatkinson
Copy link
Owner

Should be good assuming it builds here

Copy link
Owner

@sdatkinson sdatkinson left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻

@sdatkinson sdatkinson merged commit 827f4f9 into sdatkinson:main Jul 2, 2023
@olilarkin olilarkin deleted the rework-bitmap-controls branch September 2, 2023 06:02
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.

3 participants