phrack / ShootOFF

A virtual shooting range to enhance laser dry fire training.
http://shootoffapp.com
GNU General Public License v3.0
133 stars 71 forks source link

Handle processing delay to correctly identifying hits on moving targets #416

Open cbdmaul opened 8 years ago

cbdmaul commented 8 years ago

Since this has already come up multiple times and will come up again in the future, we need to have a general solution for handling the delay between the state of targets in the arena area and when a shot during that time is actually processed.

I don't think that trailing invisible targets is a scalable or reliable method for this. It isn't necessarily just motion that could cause this problem. For instance, a target disappearing rather than moving. Or pieces of an animated target moving. In all the ways a target could change, I don't think a trailing target is going to be a 100% solution. This could even come up with the duel tree exercise.

cbdmaul commented 8 years ago

I don't know of any feasible shot-to-click solution, a flash game would normally assume very little input lag. The input lag I measured for us was 100-150ms. In general, projectors are much worse than monitors. We will need to encourage people to use "Game mode" or whatever equivalent their projector has.

Edit: to clarify, that was our delay without shot processing. With native shot detection, we will process every frame in turn so the delay only applies without shot processing anyway. That processing includes all the delays of javafx to update the arena area.

phrack commented 8 years ago

For what it's worth, you don't need to put projectors like mine (meant for gaming) into a special mode, it just works right out of the box. I get how this is a problem, but not being a projector expert I never would have noticed this because it's just not a problem for my set-up. The problem I had was that the games were playing themselves because moving targets were detected as shots, which were consistently converted into clicks that achieved the desired effect (e.g. dead bad guy) .

cbdmaul commented 8 years ago

We'll leave that to #329. This is more relevant to high speed motion exercises, like clay shooting.

cbdmaul commented 8 years ago

Can you store the target shapes for some amount of time, so that the addShot can be changed to something like shot happened z milliseconds ago with location x,y?

Is there an efficient way to do this? You could have a vectors (mathematical not programming) for the movement of each target, but that seems overly complex. You could store it in a list every time a target moves at all, but that would grow huge I would think. You could do what I was trying to do with the masking, which is take samples every x milliseconds.

phrack commented 8 years ago

What about this:

  1. Every time setPosition is called in the target class save the timestamp in an array list (will necessarily be ordered since time is monotonic). Also save this timestamp in a hashmap as the key with the pre-adjusted point as the value.
  2. Move most of the checkHit implementation into the Target class. This should be done in any case for better design and I'll open an issue for it.
  3. With arena shots, pass in the required time adjustment.
  4. With the shot timestamp minus the adjustment, use a variation of a binary search (very efficient for an already sorted list) to find the appropriate timestamp value in the array list to use as the key into the points hashmap.
  5. Use the looked up points to make adjustments when checking for a hit.

This would be pretty time efficient, just using a lot of memory for targets that move many times. We don't use all that much memory now (average about 1.3 GB with several targets on the arena). Would maybe just want to limit the size of the list and map and cut them when they get about some big size.

ifly53e commented 8 years ago

Trying to implement this but I got stuck...in keeping with the numbered steps above: 1a. Added to Target.java: public List timeList = new ArrayList(); public Map<Long, Point2D> timeMap = new HashMap<Long, Point2D>();

1b. Added to setPosition in Target.java @Override public void setPosition(double x, double y) { targetGroup.setLayoutX(x); targetGroup.setLayoutY(y);

    long unModifiedPosTime = System.currentTimeMillis();
    if(timeList.add(unModifiedPosTime)){
        timeMap.put(unModifiedPosTime,new Point2D(x,y));
    }

2a. Completed by Phrack

3a. Added to TargetView.java private static AutoCalibrationManager autoCalibrationManager; private static final long frameDelay = autoCalibrationManager.getFrameDelayResult();

4a. Added binary search function to TargetView.java public Long closest(Long of, List in) { Long min = Long.MAX_VALUE; Long closest = of; for (Long v : in) { final Long diff = Math.abs(v - of); if (diff < min) { min = diff; closest = v; } } return closest; }

5a. Added code to TargetView.java in isHit() for looking up adjusted shot time @Override public Optional isHit(Shot shot) { //this shot has a time that it was processed //the system has a time called frame delay //subtract the frame delay to find the new time //search for the targets x and y associated with that new time //replace target.x and target.y with new.x and new.y //check for the hit Long adjustedTime = shot.getTimestamp() - frameDelay; Long closestTime = closest(adjustedTime,timeList); double adjTargetX = timeMap.get(closestTime).getX(); double adjTargetY = timeMap.get(closestTime).getY();

    if (targetGroup.getBoundsInParent().contains(shot.getX(), shot.getY())) {

This is where I get stuck...not sure where/how to adjust the targetGroup's X,Y in isHit()...Am I even going down the right path?

ifly53e commented 8 years ago

I could use targetgroup.setlayout() but will that redraw the group back at the old position? Do I need to create a temp target group at the adjusted position and check for the hit with the temp target group?

ifly53e commented 8 years ago

Plus the way I brought in an autocalibration manager is probably not correct...my code fails during projector start up since I check for the framedelay before it was calculated.

I was looking for a connection between the autocalibrationManager and TargetView: TargetView has a parent CanvasManager CanvasManager has a ProjectorArenaController ProjectorArenaController has a calibrationManager calibrationManager has a function getIsCalibrated...maybe I could check that before asking for the frameDelay?

cbdmaul commented 8 years ago

I'll take a look when I get back from vacation in a couple days.

cbdmaul commented 8 years ago

The easiest, but not very efficient way to do it would be to clone each target group, use setposition to change it to the position you got out of the array, then check for a hit. phrack may have better ideas.

I am concerned we are getting into the realm of video game development if we look too far into solving problems like these, as this is the type of problem you run into when handling latency in online games.

phrack commented 8 years ago

It's called "dead reckoning" in video game design, this is just the projector take on the same problem. The easiest thing to do (naive "dead reckoning") might be to estimate how far the target could have moved in x and y during frameDelay and add those distances as offset to a shot's coordinates when checking for a hit on a target. To do that it's probably best to use higher order functions in some way to customize hit detection in the Target class per target. I don't have a great idea for doing that off the top of my head, but the naive dead reckoning idea seems easy enough to test. It doesn't need to be perfect because the shot is a fuzzy red dot, just close enough to be plausible.

phrack commented 8 years ago

Having slept on it, it would be even better to have a simple overload of Target called MovingTarget that knows the frame delay and its current velocity to make these adjustments before a call to checkHit. This is still annoying enough to integrate with anything else that a PoC is needed: have some target moving around in an exercise at a constant and known velocity. Do even a sloppy hack in the existing checkHit method to see if the offset approach using this known velocity is good enough. If it is, I am willing to integrate, otherwise I don't want to spend a bunch of time on a "maybe".

ifly53e commented 8 years ago

Was there anything wrong with the proposal Phrack made on the sixth post (Mar 23)?

ArrayList is not thread safe. Should we be using a synchronized container like Vector instead? If using either container, the constant growth would be the issue...is keeping a count of 60 frames good enough? When 60 is reached, the code could remove the tail and add to the front but will the adds and removes penalty be significant?

Any idea of the projector frame delay latency time? 125ms? 100ms? What is a good estimate? We can use that constant and worry about getting it from the autocalibration manager later.

phrack commented 8 years ago

I'd pass the frame delay question to @cbdmaul. Regarding what to actually do with that measurement, we don't know the answer. There are several plausible possibilities in this thread, I'd pick the easiest one to implement even if the implementation is super sloppy and try it out. If it works we can figure out how to make it neat. If it doesn't work, we won't have wasted time writing nice code that does nothing useful.

ifly53e commented 8 years ago

I have been working this all week. In any method I choose to implement, it pretty much boils down to seeing if the shot is contained within the targetGroup's bounds. I can keep track of the targetGroup's X and Y coords and bring them back no problem. The problem is trying to clone the targetGroup. The clone method of the Object class is not available. You have to make a java class cloneable and then write a copy constructor that brings in the relevant target nodes. Since this is not an option, when I set the targetGroup's layout X and Y to the previous position based on the delay (inside isHit), the target is moved when I update to the previous X,Y while checking for the hit. This makes the target jump around on the screen as the scene updates the group after the call to setlayout. Maybe having a shadow group that gets loaded in a targetView would work, but how do you get around loading that group without a clone and if a for loop of some kind is used, cycling through each target to load this node may take up time and create delays we are correcting for in the first place.

This may work if the scene was broken down into quadrants and only the closest targets to the shot were processed instead of cycling though every target in the scene like checkHit does now but there is additional overhead associated with that as well.

cbdmaul commented 8 years ago

Why can't the copy constructor be implemented?

Also, the frame delay calculation can be re-enabled if we're a bit more careful about failing gracefully. Just having a setting for it would probably be as effective. It varies a bit due to shot detection but that should improve with native shot detection because all the java threads will be freed up.

phrack commented 8 years ago

I don't think you'll need to copy anything to make this work if you store more specific data (e.g. timestamps and coords) instead of the full targets. I also think you should focus on just doing the simplest hack to test the idea instead of going through all this trouble up front. You may find the idea doesn't even work that well and now you've put in a ton of effort for nothing. I gave a suggestion for testing this a few comments ago that should only take a few hours to try.

ifly53e commented 8 years ago

Here comes a beer bottle!

phracked: (frak-ed) verb. 1. to be insulted for not knowing as much as the arrogant leader of a project. Usage: I have just been phracked. I have been phracked before by Phrack and have just blown it off because I still want to play in Phrack's sandbox but the phrackings are starting to take their toll...

Assuming I still get to play, here is my question. This is step 5 in implementing the 23 March solution.

Current function isHit In targetView.java contains: @Override public Optional(Hit) isHit(Shot shot) { if (targetGroup.getBoundsInParent().contains(shot.getX(), shot.getY())) { //process the hit and return the Optional(Hit) } }

Assume the X,Y coordinates of the targetGroup's position 100ms ago have been found.

Please pseudocode in targetView.java (without phracking me) how to determine if the shot was within the targetGroup's previously known X,Y position. Again, the X,Y of the targetGroup's last known position has already been found and changing the X,Y of the targetGroup in isHit via setLayoutX will move the target in the displayed scene.

Assume moving the shot is not an option since the shot and the code has no idea of which target the user was shooting at.

Assume targets are accelerating and do not have a constant velocity so that real world parabolic motion can be implemented.

Thanks for helping me understand.

ifly53e commented 8 years ago

I am not at a compiler but I am going to try this next...

public Optional(Hit) isHit(Shot shot) { Group tempTargetGroup = new Group(); tempTargetGroup.Bounds = targetGroup.getBoundsInParent(); tempTargetGroup.setLayoutX = adjustedForTimeX; tempTargetGroup.setLayoutY = adjustedForTimeY;

if(targetGroup.getBoundsInParent().contains(shot.getX(), shot.getY() || tempTargetGroup.getBoundsInParent().contains(shot.getX(), shot.getY() {
    //process the hit and return the Optional(Hit) using the original targetGroup nodes
}

}

ugh...do I have to add the tempTargetGroup to the scene for contains to work properly?

ifly53e commented 8 years ago

Made progress on this tonight. Turns out a Group's bounds can not be resized so I added a rectangleRegion to the Group and added the Group to the Scene via the canvasManager. Preliminary tests with bouncingTargets are promising. More testing tomorrow with Trap and Skeet.

ifly53e commented 8 years ago

I have a hack that I have confirmed to make this issue a lot better.

(this is where trial and error took over....I first tried staying in targetView, but had to go higher to the canvasManager due to javafx thread updating issues)

So far this works for a single target with a single region.

I have not figured out what to do for multiple targets...but using my trap exercise without the added target tails gives hits when the laser hits the target. When the code is disabled, the results are degraded.

ifly53e commented 8 years ago

Based on a thought from Phrack, instead of calling the "contains" calculation in isHit, could we use a targetView function call that has a manual calculation such as:

if (abs(shot.x - adjTarget.x) < adjTarget.width && abs(shot.y - adjTarget.y) < adjTarget.height) {
    //the shot is within the bounds of the target so it is a hit
}

Is this the right logic for the manual calculation? Are the shot coords in the same coordinate system as the targetGroup's coords? Is a translation required?

Thanks for your thoughts.

ifly53e commented 8 years ago

This seemed to work but for some reason I felt like it did not work as well as hitting an actual re-positioned target. Have not tested on multiple targets yet. I kept the if statement in the checkHit function for this test.

ifly53e commented 8 years ago

All of this is trying to correct for the lag. There is still a problem with leading the target. The shot can be in the space in front of the moving target and still register as a hit. I was wondering if this would help solve that problem:

in isHit: if (targetGroup.getBoundsInParent().contains(shot.getX(), shot.getY()) && (Math.abs(shot.getFrame() - parent.get().getCameraManager().getFrameCount()) < 15 ) ){ //then process as a hit }

any thoughts?

cbdmaul commented 8 years ago

Bullet / shot travel time shouldn't be calculated arbitrarily. I'd make it a separate issue. It opens up a can of worms...we'd need to know what weapon and load, potentially calculate drop and loss of velocity, know what targets are at what distances...I wouldn't hack it in until we have an exact approach established.

ifly53e commented 8 years ago

Thanks for the response. I agree about shot travel time being a totally separate issue but I am not interested in calculating lead times. In my opinion, a shot should not be returned as a hit if it looks like it did not hit the target. How is the software supposed to know if the user shanked the shot or if the user was pulling lead on purpose? (I guess that could be a setting in the preferences...)

For projector exercises, I was wondering if after correcting for the lag (which returns the hit when it looks like the shot is on the target), a valid approach for eliminating the generation of a hit when the shot is in front of the moving target was to link the shot and the target to the frame in which the shot was generated in terms of some time in frame units (15 frames being half a second assuming a camera fps of 30). ShootOFF knows the fps so we would not have to guess.

One could also link the shot and the position of the target at the shot's frame. We could probably record the targets frame to the hash map while recording the targets position in setPosition and then see if the shot's frame is some amount of frames away from the target during isHit.

cbdmaul commented 8 years ago

I don't understand what you're trying to say. The purpose of adjusting for the delay is to sync the shot hit time to what is displayed at the time. If there is some problem with doing that, that problem should be fixed. I don't know what you're proposing. If the user has a bad trigger pull or whatever, that's the shot detection algorithm's problem to resolve.

ifly53e commented 8 years ago

I have worked another solution that improves this issue.

Still use an arraylist to record the times in setPosition and a hashmap to record the times as a key and now the Bounds of the targetGroup instead of the x,y. This way, you can just get the Bounds of past target positions and use the contains function to see if the shot is inside the bounds. I have found that a good compromise for both lead and lag is to go back 100ms in time and use those target bounds while checking for the hit in IsHit. You can completely eliminate lead (the shot is in front of the target but a hit is returned) by going back 150ms but the lag is huge (the shot is behind the target and a hit is returned). I did not bother with the using the binary search function this time because I shortened the time arraylist to 20 elements and the loop rarely goes more than 5 iterations. It appears that new bounds are put into the hashmap in 25ms to 30ms intervals so I am guessing that is the reason I can not get the bounds to completely align with the shot.

I recorded a couple videos. I will email them to you, They show the difference between the original IsHit versus the modified IsHit. The modified version makes a huge improvement for me in my moving target games.

EDIT: Forgot to mention that it works for multiple targets now that I removed the static keyword from the arraylist and hashmaps. I also use synchronized containers and locks when modifying the list or hashmap.

ifly53e commented 8 years ago
    @Override
    public void setPosition(double x, double y) {
        targetGroup.setLayoutX(x);
        targetGroup.setLayoutY(y);

        long unModifiedPosTime = System.currentTimeMillis();

        synchronized (timeList){
            if(timeList.size()>=20){
                timeList.remove(0);
            }//end if

            if(timeList.add(unModifiedPosTime)){
                synchronized (timeMap){
                    timeMap.put(unModifiedPosTime,targetGroup.getBoundsInParent());
                }//end synch timeMap
            }
            else{
                if (logger.isInfoEnabled()) logger.info("did not add to timeList.  size {} target {}",timeList.size(),this);
            }//end else
        }//end synch timelist

        if (config.isPresent() && config.get().getSessionRecorder().isPresent()) {
            config.get().getSessionRecorder().get().recordTargetMoved(cameraName, this, (int) targetGroup.getLayoutX(),
                    (int) targetGroup.getLayoutY());
        }

    }//end setPosition

    @Override
    public Optional<Hit> isHit_mod(Shot shot)  {
        logger.debug("in altered isHit");
        synchronized (timeList) {
            synchronized (timeMap){

                int counter = ((TargetView) this).timeList.size()-1;
                logger.debug("starting while loop counter is: {}",counter);
                //start from the end of the list
                while(counter >= 0){
                    //only check targets that are visible
                    //only proceed if the shot is inside the bounds
                    if(targetGroup.isVisible() && ((TargetView) this).timeMap.get(((TargetView) this).timeList.get(counter)).contains(shot.getX(),shot.getY())){
                        logger.debug("found a hit at: {}, {}, {}",((TargetView) this).timeList.get(counter), ((TargetView) this).timeMap.get(((TargetView) this).timeList.get(counter)).getMaxX(),((TargetView) this).timeMap.get(((TargetView) this).timeList.get(counter)).getMaxY());
                        logger.debug(" for shotTime:{}",shot.getTimestamp());
                        logger.debug("time difference: {}",shot.getTimestamp()-((TargetView) this).timeList.get(counter));
                        logger.debug("counter is: {}",counter);

                        //finger presses trigger -> laser hits screen -> shot detected ->  shot time recorded -> shot entered into list 
                        //target coordinates calculated -> target time recorded -> target displayed on screen (independent and moving forward as shot is processed)
                        //so get target some time in the past to prevent lead
                        //trying to prevent these cases when:
                        //user sees hit on target -> computer puts the shot behind the target and does not return a hit
                        //user sees hit in front of target -> computer puts the shot on the target and returns a hit

                        if(shot.getTimestamp()-((TargetView) this).timeList.get(counter) < 100){ //make a slider in preferences to adjust this number?
                            counter--;
                            continue;
                        }

                            // Target was hit, see if a specific region was hit
                            for (int i = targetGroup.getChildren().size() - 1; i >= 0; i--) {
                                Node node = targetGroup.getChildren().get(i);//(i);

                                //Bounds nodeBounds = targetGroup.getLocalToParentTransform().transform(node.getBoundsInParent());
                                Bounds nodeBounds = ((TargetView) this).timeMap.get(((TargetView) this).timeList.get(counter));

                                logger.debug("shot:{}, {}",shot.getX(),shot.getY());
                                logger.debug("nodeBounds:{}, {}, {}, {}",nodeBounds.getMinX(),nodeBounds.getMaxX(),nodeBounds.getMinY(),nodeBounds.getMaxY());

                                final int adjustedX = (int) (shot.getX() - nodeBounds.getMinX());
                                final int adjustedY = (int) (shot.getY() - nodeBounds.getMinY());

                                logger.debug("adjX,Y {}, {}",adjustedX, adjustedY);

                                if (nodeBounds.contains(shot.getX(), shot.getY())) {
                                    logger.debug("shot is inside the node");
                                    // If we hit an image region on a transparent pixel,
                                    // ignore it
                                    final TargetRegion region = (TargetRegion) node;

                                    // Ignore regions where ignoreHit tag is true
                                    if (region.tagExists(TargetView.TAG_IGNORE_HIT)
                                            && Boolean.parseBoolean(region.getTag(TargetView.TAG_IGNORE_HIT)))
                                        continue;

                                    if (region.getType() == RegionType.IMAGE) {
                                        logger.debug("found the image region");
                                        // The image you get from the image view is its
                                        // original size. We need to resize it if it has
                                        // changed size to accurately determine if a pixel
                                        // is transparent
                                        Image currentImage = ((ImageRegion) region).getImage();

                                        if (adjustedX < 0 || adjustedY < 0) {
                                            logger.debug(
                                                    "An adjusted pixel is negative: Adjusted ({}, {}), Original ({}, {}), "
                                                            + " nodeBounds.getMin ({}, {})",
                                                    adjustedX, adjustedY, shot.getX(), shot.getY(), nodeBounds.getMaxX(),
                                                    nodeBounds.getMinY());
                                            if (logger.isInfoEnabled()) logger.info("adjustedPixel negative");
                                            logger.debug("returned empty");
                                            return Optional.empty();
                                        }

                                        if (Math.abs(currentImage.getWidth() - nodeBounds.getWidth()) > .0000001
                                                || Math.abs(currentImage.getHeight() - nodeBounds.getHeight()) > .0000001) {

                                            BufferedImage bufferedOriginal = SwingFXUtils.fromFXImage(currentImage, null);

                                            java.awt.Image tmp = bufferedOriginal.getScaledInstance((int) nodeBounds.getWidth(),
                                                    (int) nodeBounds.getHeight(), java.awt.Image.SCALE_SMOOTH);
                                            BufferedImage bufferedResized = new BufferedImage((int) nodeBounds.getWidth(),
                                                    (int) nodeBounds.getHeight(), BufferedImage.TYPE_INT_ARGB);

                                            Graphics2D g2d = bufferedResized.createGraphics();
                                            g2d.drawImage(tmp, 0, 0, null);
                                            g2d.dispose();

                                            try {
                                                if (adjustedX >= bufferedResized.getWidth() || adjustedY >= bufferedResized.getHeight()
                                                        || bufferedResized.getRGB(adjustedX, adjustedY) >> 24 == 0) {
                                                    logger.debug("transparent region found 1");
                                                    continue;
                                                }
                                            } catch (ArrayIndexOutOfBoundsException e) {
                                                String message = String.format(
                                                        "Index out of bounds while trying to find adjusted coordinate (%d, %d) "
                                                                + "from original (%.2f, %.2f) in adjusted BufferedImage for target %s "
                                                                + "with width = %d, height = %d",
                                                        adjustedX, adjustedY, shot.getX(), shot.getY(), getTargetFile().getPath(),
                                                        bufferedResized.getWidth(), bufferedResized.getHeight());
                                                logger.error(message, e);
                                                logger.debug("index out of bounds");
                                                logger.debug("returned empty");
                                                return Optional.empty();
                                            }
                                        } else {
                                            if (adjustedX >= currentImage.getWidth() || adjustedY >= currentImage.getHeight()
                                                    || currentImage.getPixelReader().getArgb(adjustedX, adjustedY) >> 24 == 0) {
                                                logger.debug("transparent region found 2");
                                                continue;
                                            }
                                        }
                                    } else {
                                        logger.debug("did not find an image region");
                                        // The shot is in the bounding box but make sure it
                                        // is in the shape's
                                        // fill otherwise we can get a shot detected where
                                        // there isn't actually
                                        // a region showing
                                        Point2D localCoords = targetGroup.parentToLocal(shot.getX(), shot.getY());
                                        if (!node.contains(localCoords)){
                                            logger.debug("in bounding box but not in fill");
                                            continue;
                                        }else{
                                            logger.debug("not in bounding box");
                                        }
                                    }

                                    logger.debug("returned hit");
                                    return Optional.of(new Hit(this, (TargetRegion) node, adjustedX, adjustedY));

                                }//end if node contains
                        }//end for
                    }//end if group contains
                        counter--;
                    }//end while
                        logger.debug("at the end of altered isHit...returned empty");
                return Optional.empty();
            }//end sync timeMap

        }//end sync timelist

    }//end isHit_mod(shot)
cbdmaul commented 8 years ago

ifly53e, we'd need a pull request with unit tests to consider adding it at this time. We just have too much on our plates to integrate right now. The issue will remain open we're just not working on it currently.

cbdmaul commented 8 years ago

My latest commit, combined with our other autocalibration / camera improvements, makes the frame delay calculation more accurate/consistent.

At the very least, this information could be used to more accurately reflect the user's reaction time in drills such as the cowboy fast draw.

One outstanding issue for the delay measurement is that frames can be dropped if the minimal processing that happens on each frame takes too long. If we buffered them during the autocalibration then the delay measurement would be more accurate (They're in different threads)