TheDuckCow / godot-road-generator

A godot plugin for creating 3D highways and streets.
MIT License
313 stars 16 forks source link

Updated auto-texturing for one-way roads #88

Closed bdog2112 closed 1 year ago

bdog2112 commented 1 year ago

This update should take care of auto-texturing one-way roads and eliminate unwanted lane lines. All GUT tests passed.

What changed:

ROAD_POINT.GD Updated road_point.gd to use NO_MARKING texture instead of ONE_WAY in single-lane one-way situations because ONE_WAY resulted in an unwanted line on one side of the lanes where it was used.

Also, updated the "final lane" logic to insure that the "outside" lane texture is NO_MARKING in various one-way multiple lane setups.

ROAD_SEGMENT.GD Troubleshot an issue where reverse-only lane setups still showed a dotted line on the left side. Updated road_segment.gd "reverse-only" lane matching element result order.

TESTS Updated both road point and lane matching unit tests.

bdog2112 commented 1 year ago

By the way, I explicitly branched this from "dev". Not sure why it says the destination is "main".

bdog2112 commented 1 year ago

Added a fix for Issue #14 where little "kinks" sometimes appear near RoadPoints. It took research, experimentation, and hard work to come up with the solution. Initially, it was quite complex with a lengthy interpolation process. But, once the "Transform" class and "ease" function entered the picture, code shrank significantly and things quickly fell into place! It's recommended to create a new scene when testing.

TheDuckCow commented 1 year ago

By the way, I explicitly branched this from "dev". Not sure why it says the destination is "main".

It's because "main" is still the default branch, so you need to manually change the "base" to be dev (this does not modify your local branch in any way, or the remote for that matter, it's just a property of the PR itself).

Thanks for sharing the update, just tried it now and it's working well, with one way in either the reverse or forward which is great! Let me just finish up some code review.

Screen Shot 2023-04-15 at 4 01 02 PM
bdog2112 commented 1 year ago

If you have any gut reaction as to what might be the cause of that, then we can investigate; if we have no idea, or you are quite confident that it wasn't introduced by this change, then I'd say we can proceed to merge (after the other minor comments are addressed).

Regarding the quirky redraw behavior, I tested, just now, and observed the same thing in the "dev" branch. The problem is not specific to the latest updates. (Update: Just tested dev, again, and can't re-create problem. Should probably limit focus to this branch.)

When the LaneSegment branch was merged, I noticed that it only displayed errors when the Start Point was selected, which informs how I approached re-creating this particular problem. I doubt this issue has anything to do with LaneSegments. But, Start Points seem to be a key component.

Only alternate selection of the Start Points. Then, the problem is more likely to manifest. (Update: after further testing, this didn't work every time. However, I did have pretty good luck consistently re-creating the problem by always selecting the Start Point on the curved section of road immediately after re-loading the scene.)

Regarding the distorted geometry on turns, let's analyze an extreme example: Create a Segment where the two RoadPoints are only one meter apart. Then, create a 90 degree turn while maintaining the one meter distance.

The RoadPoints are too close together. There is not enough space to draw a clean turn. The solution is to move the RoadPoints further apart. We could, theoretically, design the system to draw clean turns that close. Is that what we want to do? What is the minimum acceptable distance target? Once we know that, then we can try to accommodate it.

Also, bear in mind that road engineers wouldn't design a 90 degree turn with a hard and tight 90 degree angle on the inside corner. They would create sufficient distance between the start and end points so that a car could make the turn in any lane while traveling at a reasonable rate of speed.

Lastly, these tight turns would be easier to model if the curve was positioned along the inside edge of the road. Unfortunately, the curve is in the road center. But, the center is the most flexible choice for the greatest number of use cases.

bdog2112 commented 1 year ago

Regarding the fix for road kinks, please note that we're using an arbitary hard coded value in the "ease" function. It is labeled "smooth_amount".

In previous releases, you observed that there was sometimes an unexpected rotation in the loop immediately following a RoadPoint. The "ease" function basically extends the RoadPoint rotation for an arbitrary distance (aka "smooth_amount") and then eases into the actual interpolation between Start and End points. This effectively prevents those unwanted rotations immediately after the RoadPoint.

We can shorten or lengthen the smooth_amount depending on our needs. I set it pretty low, already. But, it could potentially go even lower without too much adverse effect. I'd suggest experimenting with it.

TheDuckCow commented 1 year ago

We could, theoretically, design the system to draw clean turns that close. Is that what we want to do? What is the minimum acceptable distance target? Once we know that, then we can try to accommodate it.

At the highest level, totally agree with you - if a user creates an impossible setup, we'll end up with an impossible output. Maybe sometime in the future we could, as you say, do some more manual handling of kinked areas of curves, but it's not worth it right now for the vast amount of extra work that would take. Appreciate you entertaining the though briefly though.

Regarding the quirky redraw behavior, I tested, just now, and observed the same thing in the "dev" branch. The problem is not specific to the latest updates.

Ok that's good (in a sense), so increases my confidence of pushing this through. Appreciate your extra troubleshooting on this.

Lastly, these tight turns would be easier to model if the curve was positioned along the inside edge of the road. Unfortunately, the curve is in the road center. But, the center is the most flexible choice for the greatest number of use cases.

Funny you mention that, after having trouble sleeping one night I let my mind wander and ponder this more. I think we actually will want to generate curves which represent the outside edges of the roads. This would also be the answer to how to fix the curvature of the LaneSegments during highly curvy areas, since they don't stay their lanes well. Doing some reading, we essentially need to perform a bezier offset - but that so far really just means taking samples of an existing bezier curve, with spacing based on the rate of curvature. This can get us a new curve in the offset position which will have many points, more than the two points of the source, but more closely match the curve in question. You previously even referenced a function of the curve tool which point provide some of this info, though I think the curvature calculation needs to happen in the "offset" space as opposed to inside the center curve's space. Entirely out of scope for this task, but figured I would bring it up. Once we have the edge-aligned curves, we unlock a lot of capabilities, including adding railings, sidewalk geometry, etc all by just defining another profile similar to the roads themselves. And this time, with the nice benefit of likely using CGC generated geometry!

We can shorten or lengthen the smooth_amount depending on our needs. I set it pretty low, already. But, it could potentially go even lower without too much adverse effect. I'd suggest experimenting with it.

Thanks for flagging this. If it's just a value to tune, then I'd say let's go ahead and merge and I can see if I feel a different value is better later. From what I played with already, it was working well. But it is quite helpful for me to know that is why this solution is working well, thanks for sharing this point.

bdog2112 commented 1 year ago

Well, I posted an update further up in this thread. But, it will probably be more visible down here.

Was unable to re-create the redraw glitching in dev during a second more recent test. Also, noted that some errors in the console indicated a problem accessing a particular array index during lane matching and it seemed like it could be due to a particular reverse-only lane setup.

It's still unclear why one second it doesn't work and the next it works fine. But, it would probably be best to troubleshoot this a bit further before merging. I'll do that, tomorrow, after our meeting.

TheDuckCow commented 1 year ago

What's the status here, is there more troubleshooting that needs to be done? I recall we last landed on not being sure this wasn't a new bug being introduced. Would be great to merge this if it is ready though.

bdog2112 commented 1 year ago

The glitchiness you documented in your video was difficult to reproduce, In turn, it was difficult to troubleshoot and required a lot of trial and error as you well know.

I tried to boil it down into the simplest series of repeatable steps. However, sometimes the darn thing just worked even though it was supposed to fail. ;-) At any rate, I will describe how I re-created the problem and then offer what may be the fix. (SPOILER ALERT: The fix is to initialize RoadPoint arrays in the _init routine, not the declaration.)

Steps to re-create problem:

I tried those steps with a few different versions of the Road Generator and it seemed to consistently yield that outcome. Then, I updated road_point.gd to initialize the traffic_dir and lanes arrays in an _init routine and the problem went away.

Screenshots of test results before and after initializing arrays in the _init routine: Glitch 01 Glitch 02

bdog2112 commented 1 year ago

So, to tie a little bow around this one: the drawing glitches appear to be un-related to this PR and they should, hopefully, get resolved when you move the RoadPoint array initialization to the _init routine. Yay! Moving forward with the merge...