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

Major Refactor - Project Structure #9

Merged
merged 9 commits into from
Oct 19, 2021
Merged

Conversation

hoangvu01
Copy link
Contributor

@hoangvu01 hoangvu01 commented Oct 18, 2021

Major Project Refactor

Moved ALL functionalities/features to EngineModel which performs all logical operations for the image engine. Controllers from now on should only handle Events and then call appropriate methods in the model.

Note that there can be more than 1 model and more than 1 controller as long as their functions do not overlap (see PAC or HMVC patterns).

WIP

  • Tony's pipeline migration
  • Steven's branch

@hoangvu01 hoangvu01 changed the title Ip 13 project structure Major Refactor - Project Structure Oct 18, 2021
@hoangvu01
Copy link
Contributor Author

@xu-shitong I don't really get your pipeline. Say, you have this pipeline Grayscale > Blur > Grayscale, your pipeline will say that the Grayscale is redundant and optimise to Blur > Grayscale. But the order matters here, blurring on a grayscale image produces a different output than blurring on a coloured image. In which case should we have the pipeline as a whole seperate feature?

@xu-shitong
Copy link
Contributor

xu-shitong commented Oct 18, 2021

@xu-shitong I don't really get your pipeline. Say, you have this pipeline Grayscale > Blur > Grayscale, your pipeline will say that the Grayscale is redundant and optimise to Blur > Grayscale. But the order matters here, blurring on a grayscale image produces a different output than blurring on a coloured image. In which case should we have the pipeline as a whole seperate feature?

I remove the previous GrayScale so that when user click on the grayscale again(the grayscale box is not selected), the operation will be removed from the pipeline and the image is not in gray. if it is a non exclusive operation, like changing in RGB value, the operation will be removed from the pipeline and appended to the end of the pipeline, containing the updated RGB value
The order user click on the operations matters as you said. I was thinking about displaying the pipeline later as well, so that user have an idea of what operation they selected and in what order.

@danieldeng2
Copy link
Contributor

I think the structure is fine! We should decide on what to do together as a group first so people's changes don't overlap in the future.

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, great idea here, so basically just swap pixels in the same loop in one go and no clone is needed.

@hoangvu01 hoangvu01 marked this pull request as ready for review October 19, 2021 07:38
@hoangvu01 hoangvu01 merged commit 30353c6 into develop Oct 19, 2021
@hoangvu01 hoangvu01 deleted the IP-13-project-structure branch October 19, 2021 10:57
@JoeyTeng JoeyTeng added this to the Iteration 1 milestone Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants