frontend: Improve preview source snapping

[Warchamp7]

Description

This PR makes a number of changes to source snapping in the preview:

• Sources no longer snap to hidden sources
• Edges can now snap to the same axis edge of other sources
  • Ex. The top edge of a source can now snap to the top edge of another source
• Sources snap along the entire axis
  • Sources don't have to be touching to snap
• Visual indicators for snapping
• Replaces calculation macro with a static method

Additionally

• Changes the default snap distance from 10.0 -> 5.0
• Hidden sources can no longer be hovered or clicked in the preview to select them

obs64_wuvM7eemGG.mp4

Motivation and Context

I want to improve source snapping behaviour and visual feedback

How Has This Been Tested?

Moved a bunch of sources around. Toggled relevant snapping settings on and off to ensure behaviour matched settings.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

• Tweak (non-breaking change to improve existing functionality)

• Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[PatTheMav]

Functionally this PR is fine, but it seems very brute force and not particularly efficient to me and falls victim to "featuritis", just piling on little features one by one, each doing its own thing in isolation with no regard to other (similar) features and the architecture behind it.

Preview control drawing should be a single process, by a single responsible entity in code that knows about all of its "features" and thus set up a single render pass to draw all the pieces it knows should be visible for any given frame.

[Warchamp7]

I've added another commit that updates the default snap distance from 10.0 -> 5.0 and will migrate the old value one time. Since the snap distance is scaled by preview zoom level the default value of 10 is pretty aggressive especially with more snap candidates now.

[PatTheMav]

Is this a global type or would it make more sense to make it a nested type of OBSBasicPreview, given its limited usage?

[PatTheMav]

A function named checkSomething should not have side-effects. What is the purpose of updating the offset as part of the check?

If the update is required, there's two cleaner ways to do it:

• Either return an optional value, which is only set when the check succeeds.
• Or return a struct which contains the check result as well as updated values.

[PatTheMav]

Per our style guidelines, struct should only be used for "dumb" data structures. As soon as you add any methods to it, it needs to become a class.

I am on the fence about this - coming from Swift it feels natural to have structured data with "helper" methods embedded within the type, and as soon as you need actual heap-allocated objects which you share via references you use classes.

Probably worth amending our style guideline to make this deviation from Google's guidelines explicit, because I do not see much value in turning OffsetData into a class , just because you add helper functions to it.

[PatTheMav]

The constructor violates best practices from our style guide, because it forces the deconstruction of semantically linked values (x,y-coordinate pairs in this case) into distinct values, which opens up new ways to pass wrong data.

Indeed the code above should probably look more like this:

	if (checkEdgeSnap(movingEdge.x(), itemEdge.x(), data->clampDist, data->offset.x)) {
        vec2 startPoint {itemEdge.x(), 0.0f};
        vec2 endPoint {itemEdge.x(), screen.y};
		main->addSnapGuide({startPoint, endPoint});

The compiler will automatically pick up the desired type SnapGuide from the signature of addSnapGuide and thus take those two vec2's to create one explicitly.

The lines are shorter and easier to read, the relationship between the values is clear. Points or origins should be represented by the type system and consistently used by it. Qt itself uses QPoint for that reason (which could and maybe should(?) be used instead of vec2?).

[PatTheMav]

Suggested change

	static constexpr double kDefaultSnapDistance = 5.0;
	constexpr double kDefaultSnapDistance = 5.0;

static is superfluous.