hunkim98 / dotting

Dotting is a pixel art editor component library for react
https://hunkim98.github.io/dotting/
MIT License
43 stars 12 forks source link

Feature: optimize pan zoom #92

Closed hunkim98 closed 11 months ago

hunkim98 commented 12 months ago

🚀 [ Resolves #88 ] 🚀 [ Resolves #90 ]

Preview

Without optimization (1 million pixels)

https://github.com/hunkim98/dotting/assets/57612141/8ec6c6cf-ff88-4001-9cfa-60b70d307622

With optimization (1 million pixels)

https://github.com/hunkim98/dotting/assets/57612141/9ecb0122-5e91-431a-a5f9-c425a87d7539


Changes

Optimize panning and zooming using Offscreen Canvas


Notes

Next Up?

github-actions[bot] commented 12 months ago

PR Preview Action v1.4.4 :---: Preview removed because the pull request was closed. 2023-11-11 14:08 UTC

hunkim98 commented 11 months ago

@Lee-Si-Yoon I have attempted to implement Worker logic to our Dotting. However, there were many synchronization problems due to the library having multiple canvases. Worker logic is asynchronous and thus, it seemed that many of our canvases did not sync together. Thus I decided to discard the Worker logic and stick to our original version of simply rendering all by javascript logic.

In the future, I believe implement worker logic will be impossible for Dotting due to its design of having multiple canvases 😥

hunkim98 commented 11 months ago

@Lee-Si-Yoon I have attempted to implement Worker logic to our Dotting. However, there were many synchronization problems due to the library having multiple canvases. Worker logic is asynchronous and thus, it seemed that many of our canvases did not sync together. Thus I decided to discard the Worker logic and stick to our original version of simply rendering all by javascript logic.

In the future, I believe implement worker logic will be impossible for Dotting due to its design of having multiple canvases 😥

However I do believe we can add a worker logic to the DataLayer render where it renders through all the rows and data. However, I believe that is another issue unrelated with optimizing pan zoom, so I will not add any code using worker logic in this PR

Lee-Si-Yoon commented 11 months ago

I agree that implementing worker api is out of scope for this PR. Nice job let's merge this

hunkim98 commented 11 months ago

I agree that implementing worker api is out of scope for this PR. Nice job let's merge this

  • discussed offline about capturedImageBitmapScale getting not initialized
  • just one last complain with responsibility, do we have to create fake canvas every time when onInteractionEnded is called

I fixed the issue of capturedImageBtimapScale not being set correctly. Regarding creating fake canvas every time onInteractionEnded is called, I believe the logic would be more complex if we tried to separate parts for updating fake canvas and rendering pixel squares in them. Instead, I selectively rendered data layer and updated captured image only when data has been changed!

  onInteractionEnded(evt: TouchyEvent) {
    evt.preventDefault();
    const isDataModified = this.relayInteractionDataToDataLayer();
    if (this.mouseMode !== MouseMode.NULL && isDataModified) {
      this.dataLayer.render();
      this.dataLayer.updateCapturedImageBitmap();
    }
sonarcloud[bot] commented 11 months ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 16 Code Smells

74.5% 74.5% Coverage
15.3% 15.3% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint