Skip to content

REPLAY- 1893: Implement bitmap downscaling#1546

Merged
jonathanmos merged 1 commit into
feature/base64-jmoskovichfrom
jmoskovich/replay-1893/image-downscaling
Jul 24, 2023
Merged

REPLAY- 1893: Implement bitmap downscaling#1546
jonathanmos merged 1 commit into
feature/base64-jmoskovichfrom
jmoskovich/replay-1893/image-downscaling

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Jul 16, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Implement bitmap downscaling similar to what was done on iOS, aiming to end up with a bitmap of 10kb or less. We calculate the expected dimensions sizes for a perfect square with the required filesize, and then work out the dimensions of the resized bitmap from that.

Things to consider:

  • We are later compressing the bitmaps to webp and this gives additional savings in file size, but it's difficult to predict in advance how much the savings will be. Should we be taking this into account in the scaling of the raw bitmap?

Some numbers from testing:
Initial bitmap size: 26544
Size of bitmap after downscaling: 9400
After webp compression: 3136

Motivation

This pr will allow us to more accurately size the lrucache

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@jonathanmos jonathanmos changed the title Implement bitmap downscaling REPLAY- 1893: Implement bitmap downscaling Jul 16, 2023
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1893/image-downscaling branch from 26787a1 to 86eb5b5 Compare July 16, 2023 12:13
@codecov-commenter

codecov-commenter commented Jul 16, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.40%. Comparing base (f8cc3e6) to head (8a461d6).

❗ Current head 8a461d6 differs from pull request most recent head 5e966bf

Please upload reports for the commit 5e966bf to get more accurate results.

Additional details and impacted files 
@@                      Coverage Diff                      @@
##           feature/base64-jmoskovich    #1546      +/-   ##
=============================================================
- Coverage                      83.61%   83.40%   -0.22%     
=============================================================
  Files                            439      439              
  Lines                          14950    14991      +41     
  Branches                        2259     2258       -1     
=============================================================
+ Hits                           12500    12502       +2     
- Misses                          1878     1901      +23     
- Partials                         572      588      +16
Files with missing lines Coverage Δ
...eplay/internal/recorder/base64/Base64Serializer.kt 87.88% <100.00%> (-0.53%) ⬇️
...play/internal/recorder/mapper/ImageButtonMapper.kt 96.72% <100.00%> (+5.29%) ⬆️
...replay/internal/recorder/wrappers/BitmapWrapper.kt 20.00% <ø> (ø)
...roid/sessionreplay/internal/utils/DrawableUtils.kt 100.00% <100.00%> (+9.09%) ⬆️

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jonathanmos jonathanmos changed the title REPLAY- 1893: Implement bitmap downscaling [WIP] REPLAY- 1893: Implement bitmap downscaling Jul 17, 2023
@jonathanmos jonathanmos changed the title [WIP] REPLAY- 1893: Implement bitmap downscaling REPLAY- 1893: Implement bitmap downscaling Jul 17, 2023
@jonathanmos
jonathanmos marked this pull request as ready for review July 17, 2023 12:11
@jonathanmos
jonathanmos requested a review from a team as a code owner July 17, 2023 12:11
0xnm
0xnm reviewed Jul 17, 2023
View reviewed changes

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I left one comment wondering if we can use only 1 bitmap during the image conversion flow, I guess it should be possible.

Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt
Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1893/image-downscaling branch from cfb32f5 to 7ca6298 Compare July 18, 2023 13:10
@jonathanmos
jonathanmos requested review from 0xnm and mariusc83 July 19, 2023 11:54
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1893/image-downscaling branch from 0d40dc0 to 99c2142 Compare July 19, 2023 13:07
mariusc83
mariusc83 approved these changes Jul 19, 2023
View reviewed changes

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

0xnm
0xnm reviewed Jul 19, 2023
View reviewed changes
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1893/image-downscaling branch 2 times, most recently from 8fbe3a3 to 8a461d6 Compare July 23, 2023 10:55
@jonathanmos
jonathanmos requested a review from 0xnm July 24, 2023 12:38
0xnm
0xnm approved these changes Jul 24, 2023
View reviewed changes
Comment thread .../main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/ImageButtonMapper.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1893/image-downscaling branch from 7ab5589 to 5e966bf Compare July 24, 2023 13:04
@jonathanmos
jonathanmos merged commit dcd6805 into feature/base64-jmoskovich Jul 24, 2023
@jonathanmos
jonathanmos deleted the jmoskovich/replay-1893/image-downscaling branch July 24, 2023 13:22
@xgouchet xgouchet added this to the 2.2.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@0xnm 0xnm 0xnm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@jonathanmos @codecov-commenter @mariusc83 @0xnm @xgouchet