Skip to content

Fixed drawingResize dimensions calculation #2123

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

Open
wants to merge 22 commits into
base: v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ebd2423
[BUGFIX]: Removed size correction based on offset
R3dByt3 Apr 28, 2025
860f88e
[BUGFIX]: Fixed aspect ratio calculation of drawing resize
R3dByt3 Apr 28, 2025
9691bfb
[BUGFIX]: Fixed row pt to px conversion
R3dByt3 Apr 28, 2025
d8500dc
[SUGGESTION]: Updated default col width calculation
R3dByt3 Apr 28, 2025
bb9e855
[MISC]: Removed unused import
R3dByt3 Apr 28, 2025
f430024
[BUGFIX]: Added x1 and y1 as return values of positionObjectPixels again
R3dByt3 Apr 28, 2025
3eda89c
[BUGFIX]: Picture anchor was not respecting calculated start offsets
R3dByt3 Apr 28, 2025
39f85ff
[BUGFIX]: Drawing anchor was not respecting calculated start offsets
R3dByt3 Apr 28, 2025
67fa421
[BUGFIX]: Shape anchor was not respecting calculated start offsets
R3dByt3 Apr 28, 2025
cf2102b
[BUGFIX]: Discarding calculated start offsets for vml
R3dByt3 Apr 28, 2025
7d6ecef
[MISC]: Adjust should respect oneCellAnchors
R3dByt3 Apr 30, 2025
8886fea
[MISC]: Adjust should respect oneCellAnchors
R3dByt3 Apr 30, 2025
1a8887c
[MISC]: Updated excel default values [BUGFIX]: Fixed drawing dimensions
R3dByt3 Apr 30, 2025
2531329
[MISC]: Updated tests
R3dByt3 Apr 30, 2025
813e767
[MISC]: Updated tests
R3dByt3 Apr 30, 2025
ddf3970
[MISC]: Fixed drawing dimensions
R3dByt3 Apr 30, 2025
0340d13
[BUGFIX]: Implemented oneCellAnchors as oneCellAnchors
R3dByt3 Apr 30, 2025
ec177c1
[MISC]: Updated tests
R3dByt3 Apr 30, 2025
c5f7cc5
[MISC]: Added missing no custom row height logic
R3dByt3 Apr 30, 2025
49d9d83
[MISC]: Updated tests
R3dByt3 Apr 30, 2025
fa62cc4
[MISC]: Updated api
R3dByt3 Apr 30, 2025
bf62569
Update vml.go
R3dByt3 Apr 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions adjust.go
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ func (from *xlsxFrom) adjustDrawings(dir adjustDirection, num, offset int, editA

// adjustDrawings updates the ending anchor of the two cell anchor pictures
// and charts object when inserting or deleting rows or columns.
func (to *xlsxTo) adjustDrawings(dir adjustDirection, num, offset int, editAs string, ok bool) error {
func (to *xlsxTo) adjustDrawings(dir adjustDirection, num, offset int, ok bool) error {
if dir == columns && to.Col+1 >= num && to.Col+offset >= 0 && ok {
if to.Col+offset >= MaxColumns {
return ErrColumnNumber
Expand All @@ -1054,27 +1054,35 @@ func (to *xlsxTo) adjustDrawings(dir adjustDirection, num, offset int, editAs st
// inserting or deleting rows or columns.
func (a *xdrCellAnchor) adjustDrawings(dir adjustDirection, num, offset int) error {
editAs := a.EditAs
if a.From == nil || a.To == nil || editAs == "absolute" {
if (a.From == nil && (a.To == nil || a.Ext == nil)) || editAs == "absolute" {
return nil
}
ok, err := a.From.adjustDrawings(dir, num, offset, editAs)
if err != nil {
return err
}
return a.To.adjustDrawings(dir, num, offset, editAs, ok || editAs == "")
if a.To != nil {
return a.To.adjustDrawings(dir, num, offset, ok || editAs == "")
} else {
Copy link
Member

Choose a reason for hiding this comment

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

The else statement no longer required. This can be simplify with:

if a.To != nil {
	return a.To.adjustDrawings(dir, num, offset, ok || editAs == "")
}
return err

Copy link
Author

Choose a reason for hiding this comment

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

Will do this 👍

return nil
}
}

// adjustDrawings updates the existing two cell anchor pictures and charts
// object when inserting or deleting rows or columns.
func (a *xlsxCellAnchorPos) adjustDrawings(dir adjustDirection, num, offset int, editAs string) error {
if a.From == nil || a.To == nil || editAs == "absolute" {
if (a.From == nil && (a.To == nil || a.Ext == nil)) || editAs == "absolute" {
return nil
}
ok, err := a.From.adjustDrawings(dir, num, offset, editAs)
if err != nil {
return err
}
return a.To.adjustDrawings(dir, num, offset, editAs, ok || editAs == "")
if a.To != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Author

Choose a reason for hiding this comment

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

Will do this 👍

return a.To.adjustDrawings(dir, num, offset, ok || editAs == "")
} else {
return nil
}
}

// adjustDrawings updates the pictures and charts object when inserting or
Expand Down Expand Up @@ -1128,6 +1136,11 @@ func (f *File) adjustDrawings(ws *xlsxWorksheet, sheet string, dir adjustDirecti
return err
}
}
for _, anchor := range wsDr.OneCellAnchor {
if err = anchorCb(anchor); err != nil {
return err
Copy link
Member

Choose a reason for hiding this comment

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

Need to add unit test for coverage this line.

Copy link
Author

Choose a reason for hiding this comment

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

There is at least already an integration test for this, which causes a failure if it is missing, adjust_test.go l. 1209 and following would fail now without this, since the col / row insertions would not occur for oneCellAnchors - if this is sufficient for you.

}
}
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions adjust_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ func TestAdjustDrawings(t *testing.T) {
assert.NoError(t, f.InsertRows("Sheet1", 15, 1))
cells, err := f.GetPictureCells("Sheet1")
assert.NoError(t, err)
assert.Equal(t, []string{"D3", "D13", "B21"}, cells)
assert.Equal(t, []string{"D3", "B21", "D13"}, cells)
wb := filepath.Join("test", "TestAdjustDrawings.xlsx")
assert.NoError(t, f.SaveAs(wb))

Expand All @@ -1215,7 +1215,7 @@ func TestAdjustDrawings(t *testing.T) {
assert.NoError(t, f.RemoveRow("Sheet1", 1))
cells, err = f.GetPictureCells("Sheet1")
assert.NoError(t, err)
assert.Equal(t, []string{"C2", "C12", "B21"}, cells)
assert.Equal(t, []string{"C2", "B21", "C12"}, cells)

// Test adjust existing pictures on inserting columns and rows
f, err = OpenFile(wb)
Expand All @@ -1227,7 +1227,7 @@ func TestAdjustDrawings(t *testing.T) {
assert.NoError(t, f.InsertRows("Sheet1", 16, 1))
cells, err = f.GetPictureCells("Sheet1")
assert.NoError(t, err)
assert.Equal(t, []string{"F4", "F15", "B21"}, cells)
assert.Equal(t, []string{"F4", "B21", "F15"}, cells)

// Test adjust drawings with unsupported charset
f, err = OpenFile(wb)
Expand Down
4 changes: 2 additions & 2 deletions chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ func TestChartSize(t *testing.T) {
t.FailNow()
}

if !assert.Equal(t, 14, anchor.To.Col, "Expected 'to' column 14") ||
!assert.Equal(t, 29, anchor.To.Row, "Expected 'to' row 29") {
if !assert.Equal(t, 11, anchor.To.Col, "Expected 'to' column 11") ||
!assert.Equal(t, 27, anchor.To.Row, "Expected 'to' row 27") {

t.FailNow()
}
Expand Down
77 changes: 37 additions & 40 deletions col.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ package excelize
import (
"bytes"
"encoding/xml"
"math"
"strconv"
"strings"

Expand All @@ -23,10 +22,10 @@ import (

// Define the default cell size and EMU unit of measurement.
const (
defaultColWidth float64 = 9.140625
defaultColWidthPixels float64 = 64
defaultRowHeight float64 = 15
defaultRowHeightPixels float64 = 20
defaultColWidth float64 = 10.5
defaultColWidthPixels float64 = 84.0
defaultRowHeight float64 = 15.6
defaultRowHeightPixels float64 = 20.8
EMU int = 9525
)

Expand Down Expand Up @@ -619,42 +618,40 @@ func flatCols(col xlsxCol, cols []xlsxCol, replacer func(fc, c xlsxCol) xlsxCol)
// rowEnd # Row containing bottom right corner of object.
// y2 # Distance to bottom of object.
//
// width # Width of object frame.
// height # Height of object frame.
func (f *File) positionObjectPixels(sheet string, col, row, x1, y1, width, height int) (int, int, int, int, int, int) {
// x2 # Width of object frame.
// y2 # Height of object frame.
func (f *File) positionObjectPixels(sheet string, positioning string, col, row, x1, y1, x2, y2 int) (int, int, int, int, int, int, int, int) {
colIdx, rowIdx := col-1, row-1
// Adjust start column for offsets that are greater than the col width.
for x1 >= f.getColWidth(sheet, colIdx+1) {
colIdx++
x1 -= f.getColWidth(sheet, colIdx)
}

// Adjust start row for offsets that are greater than the row height.
for y1 >= f.getRowHeight(sheet, rowIdx+1) {
rowIdx++
y1 -= f.getRowHeight(sheet, rowIdx)
}

// Initialized end cell to the same as the start cell.
colEnd, rowEnd := colIdx, rowIdx

width += x1
height += y1
if positioning == "" || positioning == "twoCell" {
// Using a twoCellAnchor, the maximum possible offset is limited by the
// "from" cell dimensions. If these were to be exceeded the "toPoint" would
// be calculated incorrectly, since the requested "fromPoint" is not possible

// Subtract the underlying cell widths to find end cell of the object.
for width >= f.getColWidth(sheet, colEnd+1) {
colEnd++
width -= f.getColWidth(sheet, colEnd)
}
x1 = min(x1, f.getColWidth(sheet, col))
y1 = min(y1, f.getRowHeight(sheet, row))

// Subtract the underlying cell heights to find end cell of the object.
for height >= f.getRowHeight(sheet, rowEnd+1) {
rowEnd++
height -= f.getRowHeight(sheet, rowEnd)
}
x2 += x1
y2 += y1
// Subtract the underlying cell widths to find end cell of the object.
for x2 >= f.getColWidth(sheet, colEnd+1) {
colEnd++
x2 -= f.getColWidth(sheet, colEnd)
}

// Subtract the underlying cell heights to find end cell of the object.
for y2 >= f.getRowHeight(sheet, rowEnd+1) {
rowEnd++
y2 -= f.getRowHeight(sheet, rowEnd)
}
} else {
x2 += x1
y2 += y1
}
// The end vertices are whatever is left from the width and height.
return colIdx, rowIdx, colEnd, rowEnd, width, height
return colIdx, rowIdx, colEnd, rowEnd, x1, y1, x2, y2
}

// getColWidth provides a function to get column width in pixels by given
Expand All @@ -664,13 +661,14 @@ func (f *File) getColWidth(sheet string, col int) int {
ws.mu.Lock()
defer ws.mu.Unlock()
if ws.Cols != nil {
var width float64
width := -1.0
for _, v := range ws.Cols.Col {
if v.Min <= col && col <= v.Max && v.Width != nil {
width = *v.Width
break
}
}
if width != 0 {
if width != -1.0 {
return int(convertColWidthToPixels(width))
}
}
Expand Down Expand Up @@ -800,16 +798,15 @@ func (f *File) RemoveCol(sheet, col string) error {
// pixel. If the width hasn't been set by the user we use the default value.
// If the column is hidden it has a value of zero.
func convertColWidthToPixels(width float64) float64 {
var padding float64 = 5
var pixels float64
var maxDigitWidth float64 = 7
var maxDigitWidth float64 = 8
if width == 0 {
return pixels
}
if width < 1 {
pixels = (width * 12) + 0.5
return math.Ceil(pixels)
return float64(int(pixels))
}
pixels = (width*maxDigitWidth + 0.5) + padding
return math.Ceil(pixels)
pixels = (width*maxDigitWidth + 0.5)
return float64(int(pixels))
}
2 changes: 1 addition & 1 deletion col_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ func TestColWidth(t *testing.T) {
width, err = f.GetColWidth("Sheet1", "A")
assert.NoError(t, err)
assert.Equal(t, 10.0, width)
assert.Equal(t, 76, f.getColWidth("Sheet1", 1))
assert.Equal(t, 80, f.getColWidth("Sheet1", 1))

// Test set and get column width with illegal cell reference
width, err = f.GetColWidth("Sheet1", "*")
Expand Down
6 changes: 3 additions & 3 deletions drawing.go
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ func (f *File) addDrawingChart(sheet, drawingXML, cell string, width, height, rI
}
width = int(float64(width) * opts.ScaleX)
height = int(float64(height) * opts.ScaleY)
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
colStart, rowStart, colEnd, rowEnd, x1, y1, x2, y2 := f.positionObjectPixels(sheet, opts.Positioning, col, row, opts.OffsetX, opts.OffsetY, width, height)
content, cNvPrID, err := f.drawingParser(drawingXML)
if err != nil {
return err
Expand All @@ -1401,9 +1401,9 @@ func (f *File) addDrawingChart(sheet, drawingXML, cell string, width, height, rI
twoCellAnchor.EditAs = opts.Positioning
from := xlsxFrom{}
from.Col = colStart
from.ColOff = opts.OffsetX * EMU
from.ColOff = x1 * EMU
from.Row = rowStart
from.RowOff = opts.OffsetY * EMU
from.RowOff = y1 * EMU
to := xlsxTo{}
to.Col = colEnd
to.ColOff = x2 * EMU
Expand Down
62 changes: 39 additions & 23 deletions picture.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,25 +366,29 @@ func (f *File) addDrawingPicture(sheet, drawingXML, cell, ext string, rID, hyper
width = int(float64(width) * opts.ScaleX)
height = int(float64(height) * opts.ScaleY)
}
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
colStart, rowStart, colEnd, rowEnd, x1, y1, x2, y2 := f.positionObjectPixels(sheet, opts.Positioning, col, row, opts.OffsetX, opts.OffsetY, width, height)
content, cNvPrID, err := f.drawingParser(drawingXML)
if err != nil {
return err
}
twoCellAnchor := xdrCellAnchor{}
twoCellAnchor.EditAs = opts.Positioning
cellAnchor := xdrCellAnchor{}
cellAnchor.EditAs = opts.Positioning
from := xlsxFrom{}
from.Col = colStart
from.ColOff = opts.OffsetX * EMU
from.ColOff = x1 * EMU
from.Row = rowStart
from.RowOff = opts.OffsetY * EMU
to := xlsxTo{}
to.Col = colEnd
to.ColOff = x2 * EMU
to.Row = rowEnd
to.RowOff = y2 * EMU
twoCellAnchor.From = &from
twoCellAnchor.To = &to
from.RowOff = y1 * EMU
cellAnchor.From = &from

if opts.Positioning != "oneCell" {
to := xlsxTo{}
to.Col = colEnd
to.ColOff = x2 * EMU
to.Row = rowEnd
to.RowOff = y2 * EMU
cellAnchor.To = &to
}

pic := xlsxPic{}
pic.NvPicPr.CNvPicPr.PicLocks.NoChangeAspect = opts.LockAspectRatio
pic.NvPicPr.CNvPr.ID = cNvPrID
Expand Down Expand Up @@ -413,14 +417,29 @@ func (f *File) addDrawingPicture(sheet, drawingXML, cell, ext string, rID, hyper
}
pic.SpPr.PrstGeom.Prst = "rect"

twoCellAnchor.Pic = &pic
twoCellAnchor.ClientData = &xdrClientData{
if opts.Positioning == "oneCell" {
cx := x2 * EMU
cy := y2 * EMU
cellAnchor.Ext = &aExt{
Cx: cx,
Cy: cy,
}
pic.SpPr.Xfrm.Ext.Cx = cx
pic.SpPr.Xfrm.Ext.Cy = cy
}

cellAnchor.Pic = &pic
cellAnchor.ClientData = &xdrClientData{
FLocksWithSheet: *opts.Locked,
FPrintsWithSheet: *opts.PrintObject,
}
content.mu.Lock()
defer content.mu.Unlock()
content.TwoCellAnchor = append(content.TwoCellAnchor, &twoCellAnchor)
if opts.Positioning == "oneCell" {
content.OneCellAnchor = append(content.OneCellAnchor, &cellAnchor)
} else {
content.TwoCellAnchor = append(content.TwoCellAnchor, &cellAnchor)
}
f.Drawings.Store(drawingXML, content)
return err
}
Expand Down Expand Up @@ -753,18 +772,15 @@ func (f *File) drawingResize(sheet, cell string, width, height float64, opts *Gr
cellHeight += f.getRowHeight(sheet, row)
}
}
if float64(cellWidth) < width {
asp := float64(cellWidth) / width
width, height = float64(cellWidth), height*asp
}
if float64(cellHeight) < height {
asp := float64(cellHeight) / height
height, width = float64(cellHeight), width*asp
if float64(cellWidth) < width || float64(cellHeight) < height {
aspWidth := float64(cellWidth) / width
aspHeight := float64(cellHeight) / height
asp := min(aspWidth, aspHeight)
width, height = width*asp, height*asp
}
if opts.AutoFitIgnoreAspect {
width, height = float64(cellWidth), float64(cellHeight)
}
width, height = width-float64(opts.OffsetX), height-float64(opts.OffsetY)
w, h = int(width*opts.ScaleX), int(height*opts.ScaleY)
return
}
Expand Down
4 changes: 2 additions & 2 deletions picture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestAddPicture(t *testing.T) {
// Test get picture cells
cells, err := f.GetPictureCells("Sheet1")
assert.NoError(t, err)
assert.Equal(t, []string{"F21", "A30", "B30", "C30", "Q1", "Q8", "Q15", "Q22", "Q28"}, cells)
assert.Equal(t, []string{"A30", "B30", "C30", "Q1", "Q8", "Q15", "Q22", "Q28", "F21"}, cells)
assert.NoError(t, f.Close())

f, err = OpenFile(filepath.Join("test", "TestAddPicture1.xlsx"))
Expand All @@ -92,7 +92,7 @@ func TestAddPicture(t *testing.T) {
f.Drawings.Delete(path)
cells, err = f.GetPictureCells("Sheet1")
assert.NoError(t, err)
assert.Equal(t, []string{"F21", "A30", "B30", "C30", "Q1", "Q8", "Q15", "Q22", "Q28"}, cells)
assert.Equal(t, []string{"A30", "B30", "C30", "Q1", "Q8", "Q15", "Q22", "Q28", "F21"}, cells)
// Test get picture cells with unsupported charset
f.Drawings.Delete(path)
f.Pkg.Store(path, MacintoshCyrillicCharset)
Expand Down
Loading