Skip to content

修复图片压缩缩放的 bug #521

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

Merged
merged 5 commits into from
Jul 21, 2021
Merged

修复图片压缩缩放的 bug #521

merged 5 commits into from
Jul 21, 2021

Conversation

zzzgydi
Copy link
Contributor

@zzzgydi zzzgydi commented Jul 8, 2021

该PR可以修复 issue #416 的问题。

问题复现条件

  1. 一张带EXIF的jpg图片,且orientation值不为1,为2到8之间。
  2. 设置了maxWidth或者maxHeight,即触发缩放逻辑doScale

bug出现原因

当图片 exif 的 orientation 不为1,就触发了 getTransform 进行旋转矫正。getCanvas 中的 context.transform(...matrix) 影响了传递给 doScale 的 canvas。

解决方法

doScale 中采用两个新的canvas镜像交替缩放图片,而不直接采用getCanvas传递过来的canvas。(当前PR的做法)

新的修复方式是:通过context.save()以及context.restore()恢复成原来的context。

Comment on lines 137 to 142
this.clear(context, width, height)
context.save()
context.transform(...matrix)
context.drawImage(img, 0, 0)
context.restore()
resolve(canvas)
Copy link
Collaborator

Choose a reason for hiding this comment

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

实测问题似乎依旧还在?

Copy link
Collaborator

@yinxulai yinxulai Jul 15, 2021

Choose a reason for hiding this comment

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

我这边复现的测试文件:
IMG_20210626_145158.jpg.zip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

第一个commit中,我重写了doScale逻辑,多创建了一个canvas不是很好,后来回滚代码操作失误没review到。现在代码修复了一下。
如果在原项目的基础上只加上context.save()context.restore()这两句是可以修复这个问题的。如图一是没有修复的效果;图二是修复后的效果。

没有修复的效果

修复后的效果

可以看到,没有修复的效果就是图片异常,修复后的效果就是完整的。但是这里给的测试图片还是有旋转问题,因为这张图片的exif的Orientation的值为6,意味着这张图片需要旋转一下,但是这张图片在操作系统(window 10)中显示却是正的,html的img标签显示它也是正的,html的canvas显示它也是正的,要么说明了os和浏览器都已经为我们处理了exif图片的旋转问题;要么说明了这张图的原方向是逆时针90度的。可以直接二进制修改exif信息对应的Orientation为1,看到这张图原来的方向。

另外,顺便提一下,可以看到修复后的结果中图片是顺时针90度的,和Orientation为1(逆时针90度)的结果不一致。有关这一点,我看到本项目中context.transform(...matrix)中matrix的计算逻辑和网上的一些解决方案都一致,但实测下来,case 6case 8的matrix对调,才符合。以上个人拙见,不是很确定,所以有关这个暂不提PR。

Copy link
Collaborator

@yinxulai yinxulai Jul 19, 2021

Choose a reason for hiding this comment

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

Orientation 的值操作对应表:
image

更新

image

结论:

  • 实际测试 6、8 确实是反了

Copy link
Collaborator

@yinxulai yinxulai Jul 19, 2021

Choose a reason for hiding this comment

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

我的理解:

Orientation 是用来描述拍摄设备的方向,我们即使横着设备拍摄,实际上希望得到的依然是实景的方向,为了便于实际使用,设备会将拍摄时的原始图像的角度与设备的自身的角度进行一个预运算,使图片的默认方向与实景的方向一致,在 canvas 时如果再进行一次逆运算,得到的将会是设备实际拍摄角度的图像。

这里所说的处理 ios 的旋转问题可能是部分系统不会对原始图像角度与设备角度预算导致的结果

结论

无论拍摄的设备有没有进行预处理,我们强制转换成设备实际拍摄时的原始角度的图片都是不太合理的,直接使用默认的角度去对图片进行后续的处理可能是更适用的

Copy link
Contributor Author

@zzzgydi zzzgydi Jul 19, 2021

Choose a reason for hiding this comment

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

我的理解是,假如我的拍摄设备宽高是375*667。我竖着拍的话,orientation值为1,图像数据记录为375*667,此时不用处理。假如我横着拍,设备可以仍然设置orientation为1,但是图像数据记录为667*375,也就是在这一步就处理了旋转。但它也可以设置orientation为6(暂不考虑顺时针逆时针),图像数据仍然记录为375*667,也就是说它始终以设备的宽高/方向记录图像数据。

讨论横着拍时的第2种情况,用户在设备图库看这张图片,它是横着的,符合直觉,因为他就是横着拍的。这里图库(其他各种查看图片的程序)可能已经处理了该旋转问题,再呈现这张图片给用户。这时用户上传这张图片,如果浏览器不默认处理,canvas的各种图像接口也没有处理orientation,此时拿到的图像数据就会是375*667的图,在用户看来,他就有问题了。以上我们做transform就是为了修复这个问题。

有关如何确定浏览器/canvas有没有处理orientation,我感觉,最简单就是,用一张带orientation值非1的图片,使用各种图片查看程序各个os打开看看,不必纠结它应该怎么旋转,仅确定所有的图片在呈现上的方向是否一致,且是否与canvas画出来的方向一致。如果一致的话,我们就不用transform了。而这个有可能从一个通用处理,变成一个浏览器兼容性的问题。可以参考这个链接https://twitter.com/zcorpan/status/1235709107933503489 @yinxulai @nighca

Copy link
Contributor

Choose a reason for hiding this comment

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

可以参考这个链接 https://twitter.com/zcorpan/status/1235709107933503489

@zzzgydi 从这个链接以及相关的 issue(whatwg/html#4495, w3c/csswg-drafts#3799 )来看,你说的是对的;现在 规范 已经是默认基于 EXIF 信息调整显示方向,大部分浏览器也已经实现

因此在符合规范的浏览器上,目前额外的 transform 行为应该会引入问题;这么看我也赞同把这个逻辑干掉 @yinxulai

不过具体到这边不在 document 中(Out of DOM)的 canvas 以及 img 的行为,确实可能会有浏览器差异的问题(而且似乎还没有可靠的做法来判断当前浏览器是否做了 auto rotate);w3c/csswg-drafts#4666 (comment) 这边有一些信息,不过不是最新的浏览器版本;通过 https://image-orientation-test.vercel.app/ ,我本机简单地测试了下,最新版本的 safari、firefox 跟 chrome 对于 Out of DOM 的行为是一致的,因此 ever green browser 应该问题不大

不过这边具体的浏览器行为是细节了,可以单独开个 issue 来跟进这个事情

Copy link
Contributor Author

Choose a reason for hiding this comment

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

而且似乎还没有可靠的做法来判断当前浏览器是否做了 auto rotate

我实现了一个小demo: https://github.com/zzzgydi/detect-canvas-orientation

通过在库中包含一张较小的测试图片,判断浏览器是否做了处理。我目前的做法是通过检测颜色,当然也可以采用检测宽高的方式。 @nighca

Copy link
Contributor

@nighca nighca Jul 20, 2021

Choose a reason for hiding this comment

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

@zzzgydi Nice job~

我看一些别的库有通过类似的姿势来处理,比如 https://github.com/hanagejet/exif-rotate-js/pull/165/files#diff-16813a4eaa7b174b1a38e1cde21a61e98f359f353a75868506123b4a50282ebd

从实用角度来说这个是可行的,虽然可能有一些限制(比如结果是异步的);不过这个不是我上边“可靠的做法”想表达的哈,因为这个做法偏 hack(是对浏览器行为的猜测)

Copy link
Collaborator

Choose a reason for hiding this comment

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

不过这边具体的浏览器行为是细节了,可以单独开个 issue 来跟进这个事情

讨论转移:关于 canvas 绘制图像的方向兼容处理

@yinxulai yinxulai linked an issue Jul 15, 2021 that may be closed by this pull request
const factor = scale ** (1 / steps)

const mirror = document.createElement('canvas')
const mctx = mirror.getContext('2d')
const { width: originWidth, height: originHeight } = source
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件的非必要改动需要撤销 @zzzgydi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当前PR的commit记录有点难看,我关闭当前PR重新提一个吧?

Copy link
Collaborator

@yinxulai yinxulai Jul 20, 2021

Choose a reason for hiding this comment

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

不用,merge 的时候会使用 squash merge,所有的 commit 会合并成一条,不会影响主分枝。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经调整了代码,可以看看

Copy link
Collaborator

@yinxulai yinxulai left a comment

Choose a reason for hiding this comment

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

LGTM

@yinxulai yinxulai merged commit feeb257 into qiniu:master Jul 21, 2021
Copy link
Contributor

@nighca nighca left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

图片压缩,关于maxWidth或者maxHeight
4 participants