3DStreet / 3dstreet-editor

3DStreet Editor Repo
https://3dstreet.app
Other
19 stars 3 forks source link

feat: automatically save scene thumbnail when saving scene for first … #345

Closed rostyslavvnahornyi closed 10 months ago

rostyslavvnahornyi commented 11 months ago

Hello,

This is the my security rules for firebase storage

rules_version = '2';
service firebase.storage {
  match /b/{bucket}/o {
    match /scenes/{scene_uuid}/files/{allPaths=**} {
      allow read: if isPublicAccess() || isImageUnderSizeLimit();
      allow write: if request.auth != null;
    }
  }
}
function isPublicAccess() {
  return request.auth == null;
}
function isImageUnderSizeLimit() {
  return resource.size < 100 * 1024 && resource.contentType.matches('image/.*');
}
kfarr commented 10 months ago

checklist to merge:

kfarr commented 10 months ago

Working for Save existing file (that doesn't yet have a thumbnail)! - as a user opening an existing 3DStreet scene that I own that does not have a thumbnail -- this update correctly creates a new thumbnail.

Not working - "remix" existing file - as a user who has created a new scene by opening another file, such as loading streetmix scene, the thumbnail is not created when I click "remix" (equivalent to Save as...)

I tried making this work by changing line 214 in Toolbar.js from if (!doSaveAs && isImagePathEmpty) { to if (isImagePathEmpty) { but in this case it would create a new png file that is all black and I couldn't figure out why after debugging for 25 mins so I'll let that go for another day.

kfarr commented 10 months ago

Also @rostyslavvnahornyi thanks for posting the rules you're using, they're a bit out of date, the latest is here: https://github.com/3DStreet/3dstreet-editor/blob/epic-v2-save-load/public/firestore.rules#L19

Note that now a user can only save files to the path corresponding with their userid

Algorush commented 8 months ago

Not working - "remix" existing file - as a user who has created a new scene by opening another file, such as loading streetmix scene, the thumbnail is not created when I click "remix" (equivalent to Save as...)

In this case variable doSaveAs = true, from here https://github.com/3DStreet/3dstreet-editor/blob/09135a3edfe9fd02c2894f6074c6e3ed740d6de8/src/components/scenegraph/Toolbar.js#L172 and this expression is false: if (!doSaveAs && isImagePathEmpty) So the function uploadThumbnailImage is not called. In this case: if (isImagePathEmpty) { the picture turns out black because at the moment of clicking Remix, the screenshot has not yet been created for the element #screentock-destination in share screen modal. Screenshot is created at the moment the share screen window is opened. If, for example, first open the share screen window and then click remix, then the thumbnail will be at the saved scene.

Algorush commented 8 months ago

So my suggestion to fix this. Replace line 214 in src/components/scenegraph/Toolbar.js: if (!doSaveAs && isImagePathEmpty) by this if (isImagePathEmpty) { In src\components\modals\ScreenshotModal\ScreenshotModal.component.jsx, move the saveScreenshot function one level higher into the module's scope. Add code for the type='img' option to create a screenshot in the share screen modal window without saving it to a file. And call this function first in the begining of uploadThumbnailImage, before accessing the screentock-destination element. I have created PR with this: https://github.com/3DStreet/3dstreet-editor/pull/396