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

Original GD's problem: Every image is read twice (at least for DSL) #1197

Closed
shenlebantongying opened this issue Oct 6, 2023 · 2 comments · Fixed by #1217
Closed

Original GD's problem: Every image is read twice (at least for DSL) #1197

shenlebantongying opened this issue Oct 6, 2023 · 2 comments · Fixed by #1217

Comments

@shenlebantongying
Copy link
Collaborator

shenlebantongying commented Oct 6, 2023

DSL::nodeToHtml loads the image and then checks the image's size https://github.com/goldendict/goldendict/blob/1.5.0/dsl.cc#L915-L1003

DslResourceRequest::run sends the data to the browser https://github.com/goldendict/goldendict/blob/1.5.0/dsl.cc#L1912-L1955

Try this dict with single image test_dsl.zip

Set a breakpoint at File::loadFromFile, and you can see 2 calls reaches it.

image

image

Better solution: replace the size checking with CSS's image max-width.

@xiaoyifang
Copy link
Owner

Agree, use css to control the image size should be better

@shenlebantongying shenlebantongying changed the title Original GD's bug: Every image is read twice (at least for DSL) Original GD's problem: Every image is read twice (at least for DSL) Oct 7, 2023
@xiaoyifang
Copy link
Owner

xiaoyifang commented Oct 8, 2023

DSL::nodeToHtml loads the image and then checks the image's size https://github.com/goldendict/goldendict/blob/1.5.0/dsl.cc#L915-L1003

this whole logic can be removed . because there is no place to configure the maxPictureWidth, which lead the resize variable is always false.


Which means all the logic about the save/restore of maxPictureWidth can be remove.

and the custom protocol gdpicture can be removed too.(can be in another PR)

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 a pull request may close this issue.

2 participants