Alignment Graph in xpert

[sagarvijaygupta]

Fixes #238.

[sagarvijaygupta]

@marco-c What is expected as output from the alignment graph? What xpert does is it creates a list of issues. I don't think we need that? So what should I provide as the output? Same as what we have decided for the basic part ie 'y' or 'n'? So a node will be considered compatible if all its attributes match 100% and even a slight issue with one node will consider the whole webpage incompatible. Right?

[sagarvijaygupta]

// I will be matching diffY for both c1 and c2 (this is incorrect in xpert)
if(c1.isSizeDiffX() && c2.isSizeDiffY()) {
    if (testSizeDiff(c1.topAligned, c2.topAligned, c1.yError,c2.yError)) {
        issues.add(errMsg("TOP-ALIGNMENT", c1, c2));
    }

    if (testSizeDiff(c1.bottomAligned, c2.bottomAligned, c1.yError, c2.yError)) {
        issues.add(errMsg("BOTTOM-ALIGNMENT", c1, c2));
    }

    if(testSizeDiff(c1.middle, c2.middle, c1.yError, c2.yError)) {
        issues.add(errMsg("VMID-ALIGNMENT", c1, c2));
    }

    if(c1.vFill ^ c2.vFill) {
        issues.add(errMsg("VFILL", c1, c2));
    }
}

if(c1.isSizeDiffX() && c2.isSizeDiffX()) {
    if(c1.hFill ^ c2.hFill) {
        issues.add(errMsg("HFILL", c1, c2));
    }

    if (c1.centered ^ c2.centered) {
        issues.add(errMsg("CENTER-ALIGNMENT", c1, c2));
    }

    if (c1.leftJustified ^ c2.leftJustified) {
        issues.add(errMsg("LEFT-JUSTIFICATION", c1, c2));
    }

    if (c1.rightJustified ^ c2.rightJustified) {
        issues.add(errMsg("RIGHT-JUSTIFICATION", c1, c2));
    }
}
}    

Just for the record as this thing will be different from original implementation

Also there is one more thing:

1. I feel if p1 and p2 are same it should return true else it should do the given stuff and then return corresponding result.
2. Also the specified method for x and y both should be same (and preferable the what is specified in y should be done in x with changes I have mentioned)
3. Moreover we are adding to issues if testSizeDiff is returned true. But p1 && e1 < 0.8 this means that if p1 (eg. is aligned left) and e1 ((x1 - x2)/ (horizontal_width_child / 3)) is less than 0.8 (which means they are close in x direction) then we are adding to issues. While we should add to issues otherwise.

   private boolean testSizeDiff(boolean p1, boolean p2, double e1, double e2) {
   if (p1 ^ p2) {
       if (p1 && e1 < 0.8) {
           return true;
       }
       if (p2 && e2 < 0.8) {
           return true;
       }
   }
   return false;
   }

[marco-c]

@marco-c What is expected as output from the alignment graph? What xpert does is it creates a list of issues. I don't think we need that? So what should I provide as the output? Same as what we have decided for the basic part ie 'y' or 'n'? So a node will be considered compatible if all its attributes match 100% and even a slight issue with one node will consider the whole webpage incompatible. Right?

Yes, then we can change the threeshold later if we want it to be less strict.

[marco-c]

Just for the record as this thing will be different from original implementation

Also add a comment in the code explaining that it is different and why.

[marco-c]

I feel if p1 and p2 are same it should return true else it should do the given stuff and then return corresponding result.

testSizeDiff should return true if there's a difference and false if there isn't, right? So if they are the same it should return false.

[marco-c]

Moreover we are adding to issues if testSizeDiff is returned true. But p1 && e1 < 0.8 this means that if p1 (eg. is aligned left) and e1 ((x1 - x2)/ (horizontal_width_child / 3)) is less than 0.8 (which means they are close in x direction) then we are adding to issues. While we should add to issues otherwise.

Why are they doing this then? Can you post a link to where testSizeDiff is used?

[sagarvijaygupta]

testSizeDiff should return true if there's a difference and false if there isn't, right? So if they are the same it should return false.

Yeah! I realised it afterwards! Thanks.

https://github.com/gatech/xpert/blob/63dcf7ffbb3cde33becad71d8c164f5e79c9d04d/src/edu/gatech/xpert/dom/layout/AGDiff.java#L63-L65

https://github.com/gatech/xpert/blob/63dcf7ffbb3cde33becad71d8c164f5e79c9d04d/src/edu/gatech/xpert/dom/layout/AGDiff.java#L226-L237

It might be I am missing something.

private double calcError(int a, int b, int delta) {
    return ((double)Math.abs(a-b))/delta;
}

This is the error they are using a -> x1 value b -> x2 value delta -> child_width/3

[sagarvijaygupta]

@marco-c I get it. Sorry for the trouble. They are checking if one is said to be aligned and other is not, then they check how much aligned it is. If it is less than 0.8, that means that it is too much aligned and hence we have the issue since other one is not aligned. If it is not that aligned we consider it not that much aligned and skip the issue.

[codecov-io]

Codecov Report

Merging #256 into master will decrease coverage by 3.25%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
- Coverage   19.73%   16.48%   -3.26%     
==========================================
  Files          12       12              
  Lines        1520     1820     +300     
  Branches      226      311      +85     
==========================================
  Hits          300      300              
- Misses       1218     1518     +300     
  Partials        2        2

Impacted Files	Coverage Δ
dom_test.py	0% <0%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5a41b9d...1cb0092. Read the comment docs.

[sagarvijaygupta]

Some findings while testing:

1. After the first part even if all the nodes were matched, in layout we match parents of nodes and siblings.
2. Siblings create higher number of issues than parents. It can be seen in the results gathered directly running the xpert itself also.

[sagarvijaygupta]

@marco-c This PR is also ready!

[propr[bot]]

Please provide your feedback on this pull request here.

Privacy statement: We don't store any personal information such as your email address or name. We ask for GitHub authentication as an anonymous identifier to account for duplicate feedback entries and to see people specific preferences.