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

fix resolution info #550 #551

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

Roc25
Copy link

@Roc25 Roc25 commented Oct 19, 2024

Fix for #550

@CyanVoxel
Copy link
Member

Thank for the fix! I think it would be more worth it to separate out raster images into a new IMAGE_RASTER_TYPES object that helps compose IMAGE_TYPES similar to how the vector and raw images work, that way there's no need for the is_image_ext_raster() function.

@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Type: UI/UX User interface and/or user experience Status: Changes Requested Changes are quested to this labels Oct 19, 2024
@CyanVoxel CyanVoxel added this to the SQL Parity milestone Oct 19, 2024
@Roc25
Copy link
Author

Roc25 commented Oct 20, 2024

You're right it will be better. But i'm not sure must i set is_iana in IMAGE_RASTER_TYPES (Its false at this moment)

@CyanVoxel
Copy link
Member

You're right it will be better. But i'm not sure must i set is_iana in IMAGE_RASTER_TYPES (Its false at this moment)

I (roughly) go by the IANA categories for this, which seems to include things like SVGs under their "image" category - so I would have IMAGE_TYPES set to true as that should include the union of all other image types, while IMAGE_RASTER_TYPES would be set to false. This should prevent situations where leveraging the mimetype for something like an svg might result in it thinking that it's a raster image specifically, while keeping the ability for a previously unknown image type to attempt to be rendered through normal means.

@CyanVoxel CyanVoxel removed the Status: Changes Requested Changes are quested to this label Oct 26, 2024
@CyanVoxel CyanVoxel added the Status: Mergeable The code is ready to be merged label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Mergeable The code is ready to be merged Type: Bug Something isn't working as intended Type: UI/UX User interface and/or user experience
Projects
Status: 🍃 Pending Merge
Development

Successfully merging this pull request may close these issues.

2 participants