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

AddPicture Method Not Respecting AutoFit and LockAspectRatio in GraphicOptions #1536

Closed
t4traw opened this issue Apr 28, 2023 · 8 comments
Closed
Labels
confirmed This issue can be reproduced
Projects

Comments

@t4traw
Copy link

t4traw commented Apr 28, 2023

Description

I'm experiencing an issue with the AddPicture method in the excelize package. Despite setting AutoFit to true and LockAspectRatio to true in GraphicOptions, it seems that the aspect ratio is only respected in the vertical direction, not in the horizontal direction. As a result, the images are distorted and do not fit correctly within the cells.

Here's the relevant part of my code:

package main

import (
	_ "image/jpeg"

	"github.com/xuri/excelize/v2"
)

func main() {
	xlsx, _ := excelize.OpenFile("base.xlsx")
	pos, _ := xlsx.SearchSheet("Sheet1", "[photo]")
	graphicOptions := excelize.GraphicOptions{
		AutoFit:         true,
		LockAspectRatio: true,
	}
	xlsx.AddPicture("Sheet1", pos[0], "400x300.jpg", &graphicOptions)
	xlsx.SaveAs("output.xlsx")
}

Describe the results you received:

file: base.xlsx

bug_sample

file: output.xlsx

output

Output of go version:

go version go1.19 darwin/arm64

Excelize version or commit ID:

2.7.1

Environment details (OS, Microsoft Excel™ version, physical, etc.):

  • OS: macOS 13.3.1
  • Excel: 16.72

Any help on this issue would be greatly appreciated. Thank you!

@yamato1413
Copy link

yamato1413 commented May 17, 2023

I have encountered the same problem too.

picture.go:drawingResize resizes the image while maintaining the aspect ratio.
Afterwards, col.go:positionObjectPixels calculates the cell to be pasted. I could not find any errors in the logic here either.

I suspect col.go:getColWidth.

I set the A1 cell to 300x300 px in Excel and ran AddPicture.
col.go:getRowHeight output 300 px, but col.go:getColWidth output 268 px.
Based on these values, picture.go:drawingResize was executed, resulting in an image size of 268x268px. (The image size should be 300x300px)
In col.go:positionObjectPixels the Column process is calculated according to the wrong cell width. Here a 268px wide image is enlarged to a 300px cell width.
The Row process knows the correct cell height and sets a 268px image.
This results in the aspect ratio being broken.(300x268px)

go version:

go version go1.20.4 windows/amd64

excelize version:

2.7.1

os:

Windows 10 Pro 22H2

excel:

Microsoft® Excel® for Microsoft 365 MSO (version 2304 build 16.0.16327.20200) 32 bit 

@xuri
Copy link
Member

xuri commented May 18, 2023

Hi @t4traw, thanks for your issue. I have tested with your code and it seems works well. Could you provide the base.xlsx and 400x300.jpg as a attachments?

@xuri
Copy link
Member

xuri commented May 18, 2023

Hi @yamato1413, thanks for your feedback. The problem as your mentions was related to the issues #260, #279, #569 and #1419.

@t4traw
Copy link
Author

t4traw commented May 18, 2023

@xuri Thank you for response! I'm sending a zip of the code tested in my environment along with the output results (also, I've created a repository for it https://github.com/t4traw/excelize_addPicture_sample).
Thank you for such a wonderful library!

sample.zip

@xuri xuri added the confirmed This issue can be reproduced label May 18, 2023
@xuri xuri closed this as completed in 08ba272 May 18, 2023
@xuri
Copy link
Member

xuri commented May 18, 2023

Thanks for your feedback. I have fixed this issue, please try to upgrade the master branch code, and this patch will be released in the next version.

@xuri xuri added this to Bugfix in v2.8.0 May 18, 2023
@t4traw
Copy link
Author

t4traw commented May 18, 2023

@xuri Thank you! I'm looking forward to the v2.8.0!

@t4traw
Copy link
Author

t4traw commented May 19, 2023

@xuri I immediately tried out the commit contents of the master branch! It seems better than before, but it still appears the scale value is off. I've attached the re-output results as a zip file.

sample_ 08ba272.zip

export_image

xuri added a commit that referenced this issue May 19, 2023
@xuri
Copy link
Member

xuri commented May 19, 2023

Thanks for your feedback. I changed the default point to the pixels conversion factor for a better scale effect. But we still can't get the exact scaling percentage, related to I replies in issue #569, the image scaling depends on each row's height and column's width, these storage inner workbook files with pt units, we need to covert it into pixels, since this conversion was effected by device DPI, so we can't be made the conversion result fit any devices. As you open the same workbook with an image in a difference spreadsheet application, operation system, or client device, the size of an image may be different, so please adjust the scale by image options as you need.

xuri added a commit to JDavidVR/excelize that referenced this issue Jul 11, 2023
xuri added a commit to JDavidVR/excelize that referenced this issue Jul 11, 2023
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this issue Oct 22, 2023
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this issue Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed This issue can be reproduced
Projects
No open projects
v2.8.0
Bugfix
Development

No branches or pull requests

3 participants