microsoft / MixedReality-QRCode-Sample

A single repository of Mixed Reality samples in Unity.
MIT License
101 stars 29 forks source link

QRCodeManager spawn old QRCode #15

Open Morgan-6Freedom opened 2 years ago

Morgan-6Freedom commented 2 years ago

Which sample are you reporting a bug in? MixedReality-QRCode-Sample

Describe the bug When I start my app, the QRCodeManager spawn multiple QRCode in the scene that has been seen in a previous session

To Reproduce Steps to reproduce the behavior:

  1. Start the sample, look at 1 QRCode (id = 32 for example)
  2. The QRCode is displayed
  3. Quit the app
  4. Restart the app
  5. Don't look at any QRCode
  6. There is a QRCode in the scene with the id=32

Expected behavior I would expect the QRCode Maanger to spawn only the QRCode shown to the camera after the app is started

cheles commented 1 year ago

@Morgan-6Freedom can you confirm which OS build you've repro this and whether it was Windows XR or OpenXR ? also, does this happen with our latest QR code sample?

ARGs-code commented 1 year ago

I am seeing this same issue in the QR Code Sample project as well. First encountered it when adapting this sample to my project and noticing that it keeps showing QR codes from previous sessions, instead of only showing QR Codes that have been seen in a given session. The Watcher does not appear to shut off properly, even when "StopQRTracking" is called. When the application loads, it immediately adds any previously seen QR Codes, even if the application has been shut down and restarted.

The only way to fully flush the watcher appears to be restarting the system, which is not acceptable.

Issue seen when using OpenXR, MRTK 2.8, Unity 2020.3.41f1 and building for HoloLens 2.

As a final note, which is the part that seems exceptionally odd- We noticed this issue in our own application first, as noted. So we went and built the sample project to see if we could recreate it. What we saw was very odd: Upon building the sample project, on first run, it added a QR Code that was observed in our custom app into the sample project despite no QR codes being present in the physical environment.

cheles commented 1 year ago

"The only way to fully flush the watcher appears to be restarting the system, which is not acceptable" this is by design as QR codes are kept at system level https://learn.microsoft.com/en-us/windows/mixed-reality/develop/advanced-concepts/qr-code-tracking-overview#are-qr-codes-saved-at-the-space-level-or-app-level

You should implement, in your app logic, a timestamp QR check. so you ignore all old scanned QR right at app start.

Now, regarding what @Morgan-6Freedom mentioned, on step 6, I don't remember seeing this. I will re-test and get back here with my findings. was it the main branch or OpenXR one?

Though, same advice applies to this, QR timestamp check at app startup.

ARGs-code commented 1 year ago

@cheles this logic should be implemented into the sample project then. Without it there is confusing behavior, which is exactly as @Morgan-6Freedom describes. Because the sample app does not do any checking for timestamps, the sample app shows QR codes from previous sessions, because they are automatically called via the QRCodeWatcher Added Event.

cheles commented 1 year ago

@ARGs-code our sample is a QR code tracking proof-of-concept and not a ready-for-production application. we are working on updating our QR code tracking documentation https://learn.microsoft.com/en-us/windows/mixed-reality/develop/advanced-concepts/qr-code-tracking-overview#managing-qr-code-data with steps on how to check the timestamp. it should fairly straight forward if you check for LastDetectedTime in QRCodeVisualizer.cs https://learn.microsoft.com/en-us/windows/mixed-reality/develop/native/qr-code-tracking-cs-cpp#qr-code-tracking-api-reference

ARGs-code commented 1 year ago

Yes, the code required to resolve this is very straight forward and once you explained the issue and we went through the entire documentation instead of skimming it, we were able to get it working as needed very easily. Thank you for clearly explaining that this behavior is intended and how to resolve it.

As a demo though, production ready or not, I still think this behavior is confusing.

bb1121 commented 1 year ago

I am having the same issue. But I am pretty new to programing so I'm not to sure on how to fix it with the explanation that was given can anyone go into further detail on how to fix the issue?

cheles commented 1 year ago

@bb1121 one way of doing is to set a DateTimeOffset watcherStart = DateTimeOffset.Now in SetupQRTracking() and check for it in HandleEvents() in QRCodesVisualizer.cs with if (action.type == ActionData.Type.Added && action.qrCode.LastDetectedTime > QRCodesManager.Instance.watcherStart)

If qrCode is old, you can do a loop and remove it foreach (var obj in qrCodesObjectsList) { Destroy(obj.Value); } qrCodesObjectsList.Clear();

This will clear the QR queue every time you start the app.

bb1121 commented 1 year ago

Thank you for taking the time to explain it further.