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.

Sign up for GitHub

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

Jump to bottom

implemented the interface #132

Merged
merged 6 commits into from
Dec 11, 2023
Merged

implemented the interface #132

merged 6 commits into from
Dec 11, 2023

Conversation

a-miscellaneous
Copy link
Contributor

@a-miscellaneous a-miscellaneous commented Dec 7, 2023 •
edited
Loading

  • extend matching types to include perfect matches
  • implement showing pictures
  • implement init with fake sequence fetching
  • implemented init with real sequence fetching
  • implemented frame jumping
  • bound buttons to execute frame jumping
  • implemented difference jumping + bind to buttons
  • transfer paths from selectionScreen to differenceScreen
  • made all screens proportional
  • created a basic ugly Theme
  • added keyboard navigation

NEW TICKET:
extend testCaseGenerator to generate Matches and PERFECTS
choose better color scheme

@a-miscellaneous a-miscellaneous linked an issue Dec 7, 2023 that may be closed by this pull request
Implement the functionality of the frame navigation interface (Library 3) #125
Closed
@a-miscellaneous a-miscellaneous marked this pull request as draft December 7, 2023 14:07
@a-miscellaneous a-miscellaneous marked this pull request as ready for review December 8, 2023 12:30
@a-miscellaneous a-miscellaneous requested review from a team and simonsasse and removed request for a team December 8, 2023 12:44
@a-miscellaneous a-miscellaneous force-pushed the gui/feature/interface-functionality branch from 29c5047 to 967f26c Compare December 8, 2023 16:38
@akriese
Copy link
Contributor

akriese commented Dec 9, 2023 •
edited
Loading

There is quite a lot to this PR. Some things I would not consider part of the actual ticket, e.g. UI changes and changes to the DifferenceGenerator subproject. Would it be possible to take these changes and put them into a separate PR to better review this one? Because I dont really know where to start here as so many different things are happening

EDIT: I think, some of the above mentioned design decisions should be discussed in the team before being merged

@a-miscellaneous a-miscellaneous force-pushed the gui/feature/interface-functionality branch from f0bd1dd to 3d2e613 Compare December 9, 2023 15:35
simonsasse
simonsasse requested changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@simonsasse simonsasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really a huge step for the GUI ! Thank you for your effort :)
There are some things though, the documentation, some naming stuff and the TestCaseGenerator. And also it would be nice to have a cleaner commit history I guess.

DifferenceGenerator/src/main/kotlin/algorithms/AlignmentAlgorithm.kt Outdated Show resolved Hide resolved
GUI/src/main/kotlin/frameNavigation/FrameNavigation.kt Show resolved Hide resolved
GUI/src/main/kotlin/frameNavigation/FrameNavigation.kt Show resolved Hide resolved
GUI/src/main/kotlin/logic/differenceGeneratorWrapper/DifferenceGeneratorWrapper.kt Show resolved Hide resolved
GUI/src/main/kotlin/ui/components/AutoSizeText.kt Outdated
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.TextUnit

// Reimplements the Text composable with auto-sizing functionality.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use:
/** SOME DOC
*
*/

So it will be displayed if you hover over an element in the IDE :)

GUI/src/main/kotlin/ui/screens/DiffScreen.kt Outdated
import java.nio.file.Path
import frameNavigation.FrameNavigation
import ui.components.AutoSizeText
import wrappers.AllVideos

/**
* A Composable function that creates a screen to display the differences between two videos.
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add @params here :)

GUI/src/main/kotlin/ui/screens/SelectVideoScreen.kt
* @param setScreen A function that will be called if the user selected all three paths and wants to
* continue.
* @return [Unit]
*/
@Composable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc

GUI/src/main/kotlin/wrappers/AllVideos.kt Outdated Show resolved Hide resolved
DifferenceGenerator/src/test/kotlin/TestCaseGenerator.kt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem with these changes, because the generated alignment might contain MATCH elements that are not PERFECT.
So this might get more complicated now because we need to change the way we insert differences in the generated test case.

DifferenceGenerator/src/test/kotlin/TestCaseGenerator.kt
@@ -114,7 +114,7 @@ class TestCaseGenerator(
AlignmentElement.DELETION -> {
videoGenerator1.loadFrame(getByteArray(position + 1, false))
}
AlignmentElement.MATCH -> {
AlignmentElement.PERFECT -> {
videoGenerator1.loadFrame(getByteArray(position + 1, false))
videoGenerator2.loadFrame(
getByteArray(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we use a random bool to decide wether to include differences or not. But even if the bool is true, it might be that no differences will be included. This is because the folder with the modified screenshots also contains not modified ones.
To correct this, we would need to look into the folder were only modified screenshots are and check if a modified version of the current frame exists. And only if it exists, we can add it. In this case the alignmentElement would be a MATCH and not a PERFECT

simonsasse
simonsasse reviewed Dec 11, 2023
View reviewed changes
DifferenceGenerator/src/main/kotlin/algorithms/Gotoh.kt Outdated
@@ -157,7 +157,11 @@ class Gotoh<T>(
}

while (i > 0 || j > 0) {
traceback.add(origin)
if (origin == AlignmentElement.MATCH && similarityM[i][j] == 1.0) { // check indexes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// check indices

@simonsasse simonsasse mentioned this pull request Dec 11, 2023
Closed
simonsasse
simonsasse requested changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@simonsasse simonsasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny CnP error, otherwise LGTM :)

GUI/src/main/kotlin/ui/screens/DiffScreen.kt Outdated
@@ -74,6 +80,12 @@ fun DiffScreen(
}
}

/**
* A Composable function that creates a screen to display the differences between two videos.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess some CnP errors. Doc doesn't make much sense.

GUI/src/main/kotlin/ui/screens/DiffScreen.kt Outdated
@@ -82,6 +94,12 @@ fun RowScope.Title(text: String) {
)
}

/**
* A Composable function that creates a screen to display the differences between two videos.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess some CnP errors. Doc doesn't make much sense.

GUI/src/main/kotlin/ui/screens/DiffScreen.kt Outdated
@@ -95,6 +113,12 @@ fun RowScope.DisplayedImage(
}
}

/**
* A Composable function that creates a button to jump that jumps frame.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess some CnP errors. Doc doesn't make much sense.

simonsasse
simonsasse approved these changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@simonsasse simonsasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@a-miscellaneous
ignore .mov files
a4eb3ec 
Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous
added Perfect alignment element
2ca67dd 
Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous
refactored selector screen
1a08a95 
added autoscaling

Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous
implemented interface functionality
2db750e 
added lib2 wrapper

Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous
refactored difference screen
5d76044 
Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous
added pathing from selector to differences screen
1f0ceb8 
added theme

Signed-off-by: a-miscellaneous <[email protected]>
@a-miscellaneous a-miscellaneous force-pushed the gui/feature/interface-functionality branch from 16a3019 to 1f0ceb8 Compare December 11, 2023 17:48
@AlperK61 AlperK61 merged commit 07f9ec6 into main Dec 11, 2023
3 checks passed
@AlperK61 AlperK61 deleted the gui/feature/interface-functionality branch December 11, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlperK61 AlperK61 AlperK61 approved these changes

@simonsasse simonsasse simonsasse approved these changes

@akriese akriese Awaiting requested review from akriese

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement the functionality of the frame navigation interface (Library 3)
4 participants
@a-miscellaneous @akriese @simonsasse @AlperK61