Skip to content
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

Any reasons why the version 1.7.7 doesn't provide as much feedback on processd, compare to the V0.12.1? #466

Closed
ghevge opened this issue Nov 11, 2022 · 22 comments · Fixed by #520

Comments

@ghevge
Copy link

ghevge commented Nov 11, 2022

Any reasons why the version 1.7.7 doesn't provide as much feedback on processed, compare to the V0.12.1?

I'm talking about those green and blue rectangles printed in the overlaid canvas:

Quagga.onProcessed(function(result) {
		 var drawingCtx = Quagga.canvas.ctx.overlay,
            drawingCanvas = Quagga.canvas.dom.overlay;

	        if (result) {
	            if (result.boxes) {
	                drawingCtx.clearRect(0, 0, parseInt(drawingCanvas.getAttribute("width")), parseInt(drawingCanvas.getAttribute("height")));
	                result.boxes.filter(function (box) {
	                    return box !== result.box;
	                }).forEach(function (box) {
	                    Quagga.ImageDebug.drawPath(box, {x: 0, y: 1}, drawingCtx, {color: "green", lineWidth: 2});
	                });
	            }
	
	            if (result.box) {
	                Quagga.ImageDebug.drawPath(result.box, {x: 0, y: 1}, drawingCtx, {color: "#00F", lineWidth: 2});
	            }
	
	            if (result.codeResult && result.codeResult.code) {
	                Quagga.ImageDebug.drawPath(result.line, {x: 'x', y: 'y'}, drawingCtx, {color: 'red', lineWidth: 3});
	            }
	        }
		}); 
@github-actions
Copy link

Thank you for filing an issue! Please be patient. :-)

@ericblade
Copy link
Owner

I use a quite similar function with my app

const QuaggaOnProcessed = (result) => {
    const drawingCtx = Quagga.canvas.ctx.overlay;
    const drawingCanvas = Quagga.canvas.dom.overlay;
    drawingCtx.font = '24px Arial';
    drawingCtx.fillStyle = 'green';

    if (result) {
        // console.warn('* quagga onProcessed', result);
        if (result.boxes) {
            drawingCtx.clearRect(0, 0, parseInt(drawingCanvas.getAttribute('width'), 10), parseInt(drawingCanvas.getAttribute('height'), 10));
            result.boxes.filter((box) => box !== result.box).forEach((box) => {
                Quagga.ImageDebug.drawPath(box, { x: 0, y: 1 }, drawingCtx, { color: 'purple', lineWidth: 2 });
            });
        }
        if (result.box) {
            Quagga.ImageDebug.drawPath(result.box, { x: 0, y: 1 }, drawingCtx, { color: 'blue', lineWidth: 2 });
        }
        if (result.codeResult && result.codeResult.code) {
            const validated = barcodeValidator(result.codeResult.code).valid;
            Quagga.ImageDebug.drawPath(result.line, { x: 'x', y: 'y' }, drawingCtx, { color: validated ? 'green' : 'red', lineWidth: 3 });
            drawingCtx.font = '24px Arial';
            drawingCtx.fillStyle = validated ? 'green' : 'red';
            drawingCtx.fillText(`${result.codeResult.code} valid: ${validated}`, 10, 50);
            drawingCtx.fillText(result.codeResult.code, 10, 20);
            // onDetected(result);
        }
    }
};

and as far as I can tell it seems to work well. I don't use 'multiple' mode so i don't know about 'result.boxes' working, but result.box seems to work well for me as well as codeResult.

I use readers: ['upc_reader', 'ean_reader'],
for my decoder config, i'm not sure if that would make any difference. locator config is { patchSize: 'medium', halfSample: true }

If you could get an image that showcases the difference between the two, then capture the result value in the old version and in the new version, we can compare the output from the two, put the whole thing into the automated test system, and hopefully trace back to what's not working.

@ghevge
Copy link
Author

ghevge commented Nov 12, 2022

On my side, I don't seem to get the result.boxes. that is why I don't see the green rectangles for partial results. I can only see the graphics associated with the success results.

@ghevge
Copy link
Author

ghevge commented Nov 12, 2022

The old version is returning the result.boxes all the time

@ghevge
Copy link
Author

ghevge commented Nov 12, 2022

BTW I only use the "upc_reader";

@ericblade
Copy link
Owner

hmmmm. it's my understanding that there shouldn't be 'boxes' if you're not using multiple: true, but i could be wrong about that. If you could post up the result output and the image used to get there, that'd be really helpful to having a look at what changed, as well as implementing a test to ensure it doesn't change again, if it's supposed to be that way.

@ghevge
Copy link
Author

ghevge commented Nov 13, 2022

With the same configurations, the old V0.12.1 shows those boxes wile the new V1.7.7 doesn't. I would like to have the same behavior with the latest version, if possible. Showing the boxes, gives a nice feedback to the users that something is going on.

@ericblade
Copy link
Owner

If you turn on multiple: true does that output similarly to the older one?

@ghevge
Copy link
Author

ghevge commented Nov 13, 2022

With multiple: true is even worst. First the boxes are still missing + onDetected result structure is messed up

const initializedReaders = ["upc_reader"];
Quagga.init({
	    inputStream : {
	      type : "LiveStream",
	      constraints: {
	        width: {min: 640},
	        height: {min: 480},
	        facingMode: "environment",
	        aspectRatio: {min: 1, max: 2}
	      }
	    },
	    locator: {
          patchSize: "medium",
          halfSample: true
        },
        numOfWorkers: 2,
        frequency: 10,
	    decoder: {
	      readers: initializedReaders,
	      multiple: true
	    },
	    locate: true
	  }

@WizardCM
Copy link

WizardCM commented Feb 3, 2023

I can confirm in 1.7 onProcessed contains an almost constantly-empty box and boxes (even with all debug flags enabled), compared to near-constant updates in older versions. I'm now in the process of going through some past versions to track down when it was limited, and will update this comment.

Final edit: 1.4.2 correctly returns lots of boxes, 1.5.0 and above don't.

@ericblade
Copy link
Owner

Thanks for looking into that. I'll see if I can do a drop in replacement between 1.4 and 1.5 on my app if that doesn't break anything (it shouldn't?) and see what i can see... i'm not sure when i'll be able to get to that, life has been pretty nuts the last several months.

@bchr02
Copy link

bchr02 commented Sep 13, 2023

Hi @ericblade I know you have likely been really busy, but FWIW I am having the same issue and had to revert to an older version. Not sure if you are still able to look at this one. Thank you!

@ericblade
Copy link
Owner

I know I have an intent to get to all of these issues, but life has a way of getting in the way of intents. :| :|

Can you help me with some specifics as to what information it is that you're looking for specifically that is missing from the later version?

It would help, since it's apparently something I'm not using at all, since I don't notice any difference .. so it would really help me to see what you're not seeing anymore

@ghevge
Copy link
Author

ghevge commented Sep 18, 2023

@ericblade I'm trying to debug this issue, but I have a hard time with the webpack cacheing: All my quagga changes are not being refrected in compiled webpack files that the browser is using.
I've tried removing all the browser cache, removind the application data chrome folder, also tried some web developer console related settings (no cache) all this without any luck. I keep being servered the original webpack quagga version, even if in my quagga.js file I have new changes.

Do you have any tricks to remove the webpack cacheing?

I am on Chrome, running on a windows 10 system.

Thanks

PS:
the problem seem to come from this method:
key: "publishResult",

line:
resultToPublish = result.barcodes || result;

In the verions 0.x, the result.barcodes was set to undefined if not detected. In the latest version the result.barcodes is being set to empty array [], which forces the resultToPublish to be set to [] instead of the result object, which was the case in the past.

This change was probably done somewhere in the method:
getBoundingBoxes()

But I haven't digged into it yet due to the caching issue of webpack modules .

@ghevge
Copy link
Author

ghevge commented Sep 18, 2023

Also under workerThread.worker.onmessage method definition, I noticed this line was commented out:
// publishResult(e.data.result, workerThread.imageData);

I'm not 100% sure, but it seem to be required, for the feedback graphic to work properly

@ghevge
Copy link
Author

ghevge commented Sep 18, 2023

OK I've managed to figured out the cause of the caching problems: It was caused by the base64 encoded part at the end of the quagga.js file. Not sure why that section is there in the first place, as it content seems to be a duplicate of the plain text part.

I was able to test my theory, and with the following changes I manged to get the feedback back:

In publishResult method definition, replace line:
resultToPublish = result.barcodes || result;

with:

if (result.barcodes !== undefined && result.barcodes.length > 0 ) {
	resultToPublish = result.barcodes;
} else {
	resultToPublish = result;
}

And also uncomment the line:

publishResult(e.data.result, workerThread.imageData);

under workerThread.worker.onmessage

@ericblade do you want to push these changes?

@ghevge
Copy link
Author

ghevge commented Oct 10, 2023

@ericblade will it be possible for you to push the change I've mentioned above? It seems I don't have permissions to do it on this repo. Thanks!

@ericblade
Copy link
Owner

@ghevge thanks so much for having a look at that, i really had no idea what i was even looking for..

you should be able to ma ke a pull request, but i've got the source up right now, i'll go ahead and give it a spin

@ericblade
Copy link
Owner

Hmm. Looking at it, and TypeScript is telling me that none of that should work. I'm going to go ahead and @ts-ignore these lines for right now, and run it through tests and see what happens. If this does work, I'd really like to be able to build a test on what data is there now that wasn't there before, and it's going to be a bit of a mess figuring out how to make typescript accept it :D

ericblade added a commit that referenced this issue Oct 25, 2023
… versions of library), submitted from comments by @ghevge
ericblade added a commit that referenced this issue Oct 25, 2023
attempt to fix #466 (data missing in onProcessed, compared to earlier versions of library), submitted from comments by @ghevge
@ericblade
Copy link
Owner

Looks like it .. works? I don't see any runtime errors spewing, although some tests required some updates, which isn't necessarily a bad thing. I've published a new version, please let me know if that does/does not work. Thanks!

@ghevge

@WizardCM
Copy link

@ericblade I can confirm 1.8.4 fixes the documented issue. Behaviour is now near identical to 1.4.2. Your efforts are greatly appreciated @ghevge.

@ericblade
Copy link
Owner

Thanks for the feedback!

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